diff --git a/README.md b/README.md index 8fa1305..85cfb92 100644 --- a/README.md +++ b/README.md @@ -16,7 +16,7 @@ The client validates configuration and can synchronize the current ranked input ## Configuration -Copy `rumble-client.example.json` to `rumble-client.json`, set the registered `clientId`, and choose a `workDirectory` for local cache, journal, and replay evidence. Do not commit the resulting file or any token. A submission token is supplied at runtime only when issue-ops support is available. +Copy `rumble-client.example.json` to `rumble-client.json`. Ranked mode requires a registered `clientId`; practice mode may omit it. The optional `workDirectory` selects the local cache, journal, and replay-evidence root and defaults to `.rumble-client` beside the configuration file. Do not commit the resulting file or any token. A submission token is supplied at runtime only when issue-ops support is available. ## Contributing diff --git a/src/main/java/dev/robocode/rumble/client/ClientConfiguration.java b/src/main/java/dev/robocode/rumble/client/ClientConfiguration.java index e6bb4c4..8cf1cfc 100644 --- a/src/main/java/dev/robocode/rumble/client/ClientConfiguration.java +++ b/src/main/java/dev/robocode/rumble/client/ClientConfiguration.java @@ -2,6 +2,8 @@ import java.net.URI; import java.nio.file.Path; +import java.util.Objects; +import java.util.Optional; import java.util.Set; /** @@ -9,16 +11,21 @@ * * @param botsRepository reviewed bot catalog repository. * @param dataRepository Rumble data repository or its canonical predecessor. - * @param clientId registered client identity. + * @param clientId registered client identity, required only in ranked mode. * @param myBots local own-bot scheduling hints. * @param gameTypes selected ranked game types. * @param battlesPerSession maximum battles requested for one session. * @param mode local execution mode. * @param workDirectory local cache, journal, and evidence root. */ -record ClientConfiguration(URI botsRepository, URI dataRepository, String clientId, Set myBots, +record ClientConfiguration(URI botsRepository, URI dataRepository, Optional clientId, Set myBots, Set gameTypes, int battlesPerSession, ClientMode mode, Path workDirectory) { ClientConfiguration { + clientId = Objects.requireNonNull(clientId, "clientId"); + mode = Objects.requireNonNull(mode, "mode"); + if (mode == ClientMode.RANKED && clientId.isEmpty()) { + throw new IllegalArgumentException("Ranked mode requires clientId"); + } myBots = Set.copyOf(myBots); gameTypes = Set.copyOf(gameTypes); workDirectory = workDirectory.toAbsolutePath().normalize(); diff --git a/src/main/java/dev/robocode/rumble/client/ClientConfigurationLoader.java b/src/main/java/dev/robocode/rumble/client/ClientConfigurationLoader.java index 8135d36..5a4089e 100644 --- a/src/main/java/dev/robocode/rumble/client/ClientConfigurationLoader.java +++ b/src/main/java/dev/robocode/rumble/client/ClientConfigurationLoader.java @@ -14,6 +14,7 @@ import java.util.HashSet; import java.util.LinkedHashSet; import java.util.Locale; +import java.util.Optional; import java.util.Set; /** @@ -36,20 +37,32 @@ ClientConfiguration load(final Path configurationPath) throws IOException { validateSchemaVersion(configuration); final URI botsRepository = parseHttpsUri(configuration, "botsRepo"); final URI dataRepository = parseHttpsUri(configuration, "dataRepo"); - final String clientId = requiredString(configuration, "clientId"); - if (clientId.equals(EXAMPLE_CLIENT_ID)) { - throw new IllegalArgumentException("clientId must replace the example value"); - } + final ClientMode mode = parseMode(requiredString(configuration, "mode")); + final Optional clientId = parseClientId(configuration, mode); final Set myBots = parseStringSet(configuration, "myBots", false); final Set gameTypes = parseGameTypes(configuration); final int battlesPerSession = parsePositiveInteger(configuration, "battlesPerSession"); - final ClientMode mode = parseMode(requiredString(configuration, "mode")); final Path workDirectory = parseWorkDirectory(configurationPath, optionalString(configuration, "workDirectory", ".rumble-client")); return new ClientConfiguration(botsRepository, dataRepository, clientId, myBots, gameTypes, battlesPerSession, mode, workDirectory); } + private static Optional parseClientId(final JsonObject configuration, final ClientMode mode) { + final JsonElement element = configuration.get("clientId"); + if (element == null || element.isJsonNull()) { + if (mode == ClientMode.RANKED) { + throw new IllegalArgumentException("Configuration is missing clientId for ranked mode"); + } + return Optional.empty(); + } + final String clientId = requiredString(configuration, "clientId"); + if (clientId.equals(EXAMPLE_CLIENT_ID)) { + throw new IllegalArgumentException("clientId must replace the example value"); + } + return Optional.of(clientId); + } + private static JsonObject parse(final Path configurationPath) throws IOException { try { final JsonElement configuration = JsonParser.parseString(Files.readString(configurationPath)); @@ -79,6 +92,9 @@ private static URI parseHttpsUri(final JsonObject configuration, final String fi if (uri.getRawUserInfo() != null) { throw new IllegalArgumentException(fieldName + " must not contain user credentials"); } + if (uri.getRawQuery() != null || uri.getRawFragment() != null) { + throw new IllegalArgumentException(fieldName + " must not contain a query or fragment"); + } return uri; } catch (URISyntaxException exception) { throw new IllegalArgumentException(fieldName + " must be an absolute HTTPS URL", exception); diff --git a/src/main/java/dev/robocode/rumble/client/JsonContract.java b/src/main/java/dev/robocode/rumble/client/JsonContract.java index b3e56f2..dcf7ad6 100644 --- a/src/main/java/dev/robocode/rumble/client/JsonContract.java +++ b/src/main/java/dev/robocode/rumble/client/JsonContract.java @@ -75,7 +75,8 @@ URI httpsUri(final String field) { final String value = string(field); try { final URI uri = new URI(value); - if (!"https".equalsIgnoreCase(uri.getScheme()) || uri.getHost() == null || uri.getRawUserInfo() != null) { + if (!"https".equalsIgnoreCase(uri.getScheme()) || uri.getHost() == null || uri.getRawUserInfo() != null + || uri.getRawQuery() != null || uri.getRawFragment() != null) { throw invalid(document + "." + field + " must be an absolute credential-free HTTPS URL"); } return uri; diff --git a/src/main/java/dev/robocode/rumble/client/RumbleSnapshotParser.java b/src/main/java/dev/robocode/rumble/client/RumbleSnapshotParser.java index ef871ba..52fdb0c 100644 --- a/src/main/java/dev/robocode/rumble/client/RumbleSnapshotParser.java +++ b/src/main/java/dev/robocode/rumble/client/RumbleSnapshotParser.java @@ -26,7 +26,8 @@ RumbleSnapshot parse(final RepositoryReader.RepositoryCheckout checkout, final ClientConfiguration configuration) throws java.io.IOException { final EnginePin engine = parseEngine(checkout.read("engine.json"), configuration.gameTypes()); final BotCatalog catalog = parseCatalog(checkout.read("catalog.json"), configuration.botsRepository()); - final ClientRegistration registration = parseRegistration(checkout, configuration.clientId()); + final ClientRegistration registration = parseRegistration(checkout, configuration.clientId().orElseThrow( + () -> JsonContract.invalid("Ranked configuration is missing clientId"))); final Map advice = new HashMap<>(); for (final GameType gameType : configuration.gameTypes()) { final String path = "matchmaking/matches_needed-" + gameType.contractName() + ".json"; @@ -81,7 +82,8 @@ private static BotCatalog parseCatalog(final String json, final URI expectedBots continue; } final CatalogBot entry = new CatalogBot(bot.string("name"), bot.string("version"), - bot.string("platform"), bot.string("path"), matching(bot.string("sourceHash"), SHA_256, + bot.string("platform"), validatedBotPath(bot.string("path")), + matching(bot.string("sourceHash"), SHA_256, "catalog bot sourceHash must be sha256:<64 lowercase hex>")); if (activeBots.putIfAbsent(entry.displayName(), entry) != null) { throw JsonContract.invalid("catalog.json contains duplicate active bot " + entry.displayName()); @@ -209,6 +211,19 @@ private static String matching(final String value, final Pattern pattern, final return value; } + private static String validatedBotPath(final String value) { + final String[] segments = value.split("/", -1); + if (value.contains("\\") || segments.length < 2 || !segments[0].equals("bots")) { + throw JsonContract.invalid("catalog bot path must be relative to bots/"); + } + for (final String segment : segments) { + if (segment.isBlank() || segment.equals(".") || segment.equals("..")) { + throw JsonContract.invalid("catalog bot path must not contain empty or traversal segments"); + } + } + return value; + } + private static boolean sourceBelongsToRepository(final URI source, final URI repository) { final String repositoryPath = stripGitSuffix(stripTrailingSlash(repository.getPath())); final String sourcePath = source.getPath(); diff --git a/src/main/java/dev/robocode/rumble/client/RumbleSynchronizer.java b/src/main/java/dev/robocode/rumble/client/RumbleSynchronizer.java index 57ee0b4..d735ebe 100644 --- a/src/main/java/dev/robocode/rumble/client/RumbleSynchronizer.java +++ b/src/main/java/dev/robocode/rumble/client/RumbleSynchronizer.java @@ -24,6 +24,9 @@ final class RumbleSynchronizer { } RumbleSnapshot synchronize(final ClientConfiguration configuration) throws IOException { + if (configuration.mode() != ClientMode.RANKED) { + throw new IllegalArgumentException("Ranked synchronization requires ranked mode"); + } try (RepositoryReader.RepositoryCheckout checkout = openCanonical(configuration.dataRepository())) { return parser.parse(checkout, configuration); } diff --git a/src/test/java/dev/robocode/rumble/client/ClientConfigurationLoaderTest.java b/src/test/java/dev/robocode/rumble/client/ClientConfigurationLoaderTest.java index 5ff845f..7e25e4e 100644 --- a/src/test/java/dev/robocode/rumble/client/ClientConfigurationLoaderTest.java +++ b/src/test/java/dev/robocode/rumble/client/ClientConfigurationLoaderTest.java @@ -10,6 +10,7 @@ import java.io.IOException; import java.nio.file.Files; import java.nio.file.Path; +import java.util.Optional; import java.util.Set; class ClientConfigurationLoaderTest { @@ -21,7 +22,7 @@ void testUnitPositive_loadsValidPracticeConfiguration() throws IOException { final Path configurationPath = writeConfiguration("practice", "registered-client"); final ClientConfiguration configuration = loader.load(configurationPath); - assertEquals("registered-client", configuration.clientId()); + assertEquals(Optional.of("registered-client"), configuration.clientId()); assertEquals(ClientMode.PRACTICE, configuration.mode()); assertEquals(Set.of(GameType.ONE_VS_ONE, GameType.TWIN_DUEL, GameType.MELEE), configuration.gameTypes()); assertEquals(configurationPath.getParent().resolve(".rumble-client").toAbsolutePath().normalize(), @@ -36,6 +37,28 @@ void testUnitNegative_rejectsExampleClientId() throws IOException { assertThrows(IllegalArgumentException.class, () -> loader.load(configurationPath)); } + @Test + @Tag("Unit") + void testUnitPositive_allowsPracticeConfigurationWithoutClientId() throws IOException { + final Path configurationPath = Files.createTempFile("rumble-client", ".json"); + Files.writeString(configurationPath, validConfiguration("practice", "registered-client") + .replace(" \"clientId\": \"registered-client\",\n", "")); + + final ClientConfiguration configuration = loader.load(configurationPath); + + assertEquals(Optional.empty(), configuration.clientId()); + } + + @Test + @Tag("Unit") + void testUnitNegative_rejectsRankedConfigurationWithoutClientId() throws IOException { + final Path configurationPath = Files.createTempFile("rumble-client", ".json"); + Files.writeString(configurationPath, validConfiguration("ranked", "registered-client") + .replace(" \"clientId\": \"registered-client\",\n", "")); + + assertThrows(IllegalArgumentException.class, () -> loader.load(configurationPath)); + } + @Test @Tag("Unit") void testUnitNegative_rejectsUnsupportedGameType() throws IOException { @@ -66,6 +89,17 @@ void testUnitNegative_rejectsCredentialedRepositoryUrl() throws IOException { assertThrows(IllegalArgumentException.class, () -> loader.load(configurationPath)); } + @Test + @Tag("Unit") + void testUnitNegative_rejectsRepositoryUrlQueryThatCouldCarryCredentials() throws IOException { + final Path configurationPath = Files.createTempFile("rumble-client", ".json"); + Files.writeString(configurationPath, validConfiguration("ranked", "registered-client") + .replace("https://github.com/robocode-dev/rumble-data", + "https://github.com/robocode-dev/rumble-data?token=secret")); + + assertThrows(IllegalArgumentException.class, () -> loader.load(configurationPath)); + } + @Test @Tag("Unit") void testUnitNegative_rejectsEmptyGameTypes() throws IOException { diff --git a/src/test/java/dev/robocode/rumble/client/RumbleSynchronizerTest.java b/src/test/java/dev/robocode/rumble/client/RumbleSynchronizerTest.java index 0f535bc..b5ab417 100644 --- a/src/test/java/dev/robocode/rumble/client/RumbleSynchronizerTest.java +++ b/src/test/java/dev/robocode/rumble/client/RumbleSynchronizerTest.java @@ -10,6 +10,7 @@ import java.util.LinkedHashMap; import java.util.List; import java.util.Map; +import java.util.Optional; import java.util.Set; import static org.junit.jupiter.api.Assertions.assertEquals; @@ -72,8 +73,46 @@ void testRCL002_IntegrationNegative_rejectsCatalogFromDifferentBotRepository() { () -> new RumbleSynchronizer(repositories).synchronize(configuration())); } + @Test + @Tag("RCL-002") + void testRCL002_IntegrationNegative_rejectsCredentialBearingCanonicalPointer() { + final InMemoryRepositoryReader repositories = validRepositories(); + repositories.replace(PREVIOUS_REPOSITORY, "wellknown/rumble.json", + repositories.read(PREVIOUS_REPOSITORY, "wellknown/rumble.json") + .replace("https://github.com/example/rumble-data", + "https://github.com/example/rumble-data?token=secret")); + + assertThrows(IllegalArgumentException.class, + () -> new RumbleSynchronizer(repositories).synchronize(configuration())); + } + + @Test + @Tag("RCL-002") + void testRCL002_IntegrationNegative_rejectsCatalogBotPathTraversal() { + final InMemoryRepositoryReader repositories = validRepositories(); + repositories.replace(CANONICAL_REPOSITORY, "catalog.json", + repositories.read(CANONICAL_REPOSITORY, "catalog.json") + .replace("bots/java/Alpha", "bots/../Alpha")); + + assertThrows(IllegalArgumentException.class, + () -> new RumbleSynchronizer(repositories).synchronize(configuration())); + } + + @Test + @Tag("Unit") + void testUnitNegative_rejectsSynchronizationInPracticeModeBeforeRepositoryAccess() { + final InMemoryRepositoryReader repositories = validRepositories(); + final ClientConfiguration practiceConfiguration = new ClientConfiguration(BOTS_REPOSITORY, + PREVIOUS_REPOSITORY, Optional.empty(), Set.of(), Set.of(GameType.ONE_VS_ONE), 10, + ClientMode.PRACTICE, Path.of("work")); + + assertThrows(IllegalArgumentException.class, + () -> new RumbleSynchronizer(repositories).synchronize(practiceConfiguration)); + assertEquals(List.of(), repositories.requestedRepositories()); + } + private static ClientConfiguration configuration() { - return new ClientConfiguration(BOTS_REPOSITORY, PREVIOUS_REPOSITORY, "alice-desktop", Set.of(), + return new ClientConfiguration(BOTS_REPOSITORY, PREVIOUS_REPOSITORY, Optional.of("alice-desktop"), Set.of(), Set.of(GameType.ONE_VS_ONE), 10, ClientMode.RANKED, Path.of("work")); }