diff --git a/.gitignore b/.gitignore index 8e837c50..de151679 100644 --- a/.gitignore +++ b/.gitignore @@ -4,3 +4,8 @@ *target web-ui/javascript/build web-ui/javascript/report.*.json +# log4j2 writes studio-latest.log and a dated roll-over beside the working directory. The +# application has always done this when run from a checkout; a test that starts the HTTP server +# now does it too, so a test run no longer leaves untracked files behind. +studio-latest.log +studio-*.log diff --git a/README.md b/README.md index 8af0671b..5d0781fe 100644 --- a/README.md +++ b/README.md @@ -91,7 +91,7 @@ Use the operational protocol in `FIELD-VALIDATION.md` if you are writing to a re ## Testing -At the time of writing: **282 Java tests** in the standard suite and **57 JavaScript tests**, all +At the time of writing: **284 Java tests** in the standard suite and **57 JavaScript tests**, all green, on Linux and on Windows. `TESTING.md` holds the current counts, the opt-in FAT32 figure and its caveats, and is the authority — the numbers here will go stale before it does. diff --git a/README_fr.md b/README_fr.md index c83ee611..260f3acb 100644 --- a/README_fr.md +++ b/README_fr.md @@ -102,7 +102,7 @@ Utilisez le protocole opératoire de `FIELD-VALIDATION.md` si vous écrivez sur ## Tests -À l'heure où ces lignes sont écrites : **282 tests Java** dans la suite standard et **57 tests +À l'heure où ces lignes sont écrites : **284 tests Java** dans la suite standard et **57 tests JavaScript**, tous verts, sur Linux comme sur Windows. `TESTING.md` porte les comptes à jour, le chiffre de la suite FAT32 optionnelle et ses réserves, et fait autorité — les nombres cités ici se périmeront avant lui. diff --git a/TESTING.md b/TESTING.md index c7a512cc..5f94946b 100644 --- a/TESTING.md +++ b/TESTING.md @@ -75,7 +75,7 @@ Nothing here touches a device. Fixtures are synthesised in code; no device data | Suite | Tests | | --- | --- | -| Java, standard | **282**, 14 skipped — the opt-in FAT32 classes, and two link cases each of which only one platform can set up | +| Java, standard | **284**, 14 skipped — the opt-in FAT32 classes, and two link cases each of which only one platform can set up. A third conditional case, the non-loopback binding check, skips only on a machine with no routable address | | Java, with `-Dstudio.test.fat32.root=` | last measured at **172** before the C6d-5 additions; not re-measured since, because it needs the volume mounted | | JavaScript | **57** | @@ -121,6 +121,7 @@ counts. | When a conversion is proven to match its source, and every reason it is not | `ConversionVerificationTest` | `web-ui` module; **specifications** — only MATCH removes a confirmation; a path outside the library is refused rather than answered | | Where a library operation may reach: direct children only, links refused, nominal cases intact | `LibraryPathConfinementTest` | `web-ui` module; **specifications** — converted from characterization once the confinement existed. The symbolic-link case is Linux-only and the junction case Windows-only | | A conversion releases its source and its temporary even when the reader or writer throws | `ConversionStreamLifecycleTest` | `web-ui` module; **specifications** — asserts the consequence by deleting the work folder, so the three failure cases are Windows-only | +| Where the HTTP server can be reached from: the loopback answers, another address of this machine does not | `HttpServerBindingTest` | `web-ui` module; **specifications** — opens real sockets against a real server rather than reading the constant back. The refusal case needs a non-loopback address to aim at and skips where there is none | Web UI (`web-ui/javascript`, run by yarn): diff --git a/web-ui/src/main/java/studio/webui/MainVerticle.java b/web-ui/src/main/java/studio/webui/MainVerticle.java index 2a65d461..4fa1e8e3 100644 --- a/web-ui/src/main/java/studio/webui/MainVerticle.java +++ b/web-ui/src/main/java/studio/webui/MainVerticle.java @@ -33,6 +33,37 @@ public class MainVerticle extends AbstractVerticle { + /** + * The address the HTTP server binds to: the loopback, and only the loopback. + * + *

Named rather than left to {@code listen(int)}, whose default is {@code 0.0.0.0} — every + * interface. That default put an API with no authentication on the local network, where anything + * on the same segment could read, write and delete in the user's library while STUdio ran. The + * CORS filter below does not cover it: CORS is enforced by browsers, on requests issued by a + * page, and says nothing to {@code curl} or a script on another machine. + * + *

Nothing is lost by restricting it, because remote use was never possible. The web UI is + * served by this same server and addresses it as {@code http://localhost:8080}, hardcoded + * throughout the frontend, so a browser on another machine would receive the page and then send + * every request to its own loopback. The wider binding exposed the API without ever making the + * application usable from elsewhere. + * + *

Deliberately fixed and not a setting. An override would keep the exposure reachable to buy + * back a capability that does not work, and the day remote access is genuinely wanted it will + * mean changing the frontend's addresses too — which is when this decision should be revisited, + * not before. + * + *

A method rather than a {@code static final String}, so that the test which asserts this can + * read it. A compile-time constant is inlined into whatever reads it, and a test holding an + * inlined copy would go on binding to the address it was compiled against — passing while + * production had been changed under it. That is precisely the regression this must catch. + */ + static String listenHost() { + return "127.0.0.1"; + } + + static final int LISTEN_PORT = 8080; + private final Logger LOGGER = LoggerFactory.getLogger(MainVerticle.class); private DatabaseMetadataService databaseMetadataService; @@ -98,7 +129,7 @@ public void start() { }); // Start HTTP server - vertx.createHttpServer().requestHandler(router).listen(8080); + vertx.createHttpServer().requestHandler(router).listen(LISTEN_PORT, listenHost()); // Automatically open URL in browser, unless instructed otherwise String openBrowser = System.getProperty("studio.open", "true"); diff --git a/web-ui/src/test/java/studio/webui/HttpServerBindingTest.java b/web-ui/src/test/java/studio/webui/HttpServerBindingTest.java new file mode 100644 index 00000000..b3533c5b --- /dev/null +++ b/web-ui/src/test/java/studio/webui/HttpServerBindingTest.java @@ -0,0 +1,156 @@ +/* + * This Source Code Form is subject to the terms of the Mozilla Public + * License, v. 2.0. If a copy of the MPL was not distributed with this + * file, You can obtain one at https://mozilla.org/MPL/2.0/. + */ + +package studio.webui; + +import io.vertx.core.Vertx; +import io.vertx.core.http.HttpServer; +import org.junit.jupiter.api.AfterEach; +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.DisplayName; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.Assumptions; + +import java.io.BufferedReader; +import java.io.IOException; +import java.io.InputStreamReader; +import java.net.Inet4Address; +import java.net.InetAddress; +import java.net.InetSocketAddress; +import java.net.NetworkInterface; +import java.net.Socket; +import java.nio.charset.StandardCharsets; +import java.util.ArrayList; +import java.util.Collections; +import java.util.List; +import java.util.Optional; +import java.util.concurrent.CompletableFuture; +import java.util.concurrent.TimeUnit; + +import static org.junit.jupiter.api.Assertions.assertNotNull; +import static org.junit.jupiter.api.Assertions.assertThrows; +import static org.junit.jupiter.api.Assertions.assertTrue; +import static org.junit.jupiter.api.Assertions.fail; + +/** + * Where the HTTP server can be reached from. + * + *

The server used to be started with {@code listen(int)}, whose default host is {@code 0.0.0.0}: + * every machine on the same network segment could reach an API that has no authentication, and read, + * write or delete in the user's library while STUdio was running. The launcher opens a browser on + * {@code localhost}, so nothing told the user anything else was listening. + * + *

This asserts the consequence rather than the constant. It starts a real server on the address + * production uses, then opens real sockets: the loopback must answer, and a non-loopback address of + * this same machine must not. Reading {@link MainVerticle#listenHost()} back would only restate the + * source; connecting proves what the operating system actually did with it. + * + *

The refusal case needs a second local address to aim at, and a container often has none. When + * there is no non-loopback IPv4 address the case is skipped rather than passed — a machine that + * cannot set up the situation has not established anything about it. The nominal case runs + * everywhere, and carries equal weight: a server bound to nothing at all would pass every refusal. + */ +class HttpServerBindingTest { + + /** Long enough for a local connection to complete, short enough that a refusal is not a wait. */ + private static final int CONNECT_TIMEOUT_MS = 2_000; + + private Vertx vertx; + private HttpServer server; + private int port; + + @BeforeEach + void startServerOnTheProductionAddress() throws Exception { + vertx = Vertx.vertx(); + CompletableFuture started = new CompletableFuture<>(); + // Port 0, so the test never collides with a running STUdio or with a parallel run. The + // address is production's, which is the whole point. + vertx.createHttpServer() + .requestHandler(request -> request.response().end("ok")) + .listen(0, MainVerticle.listenHost(), ar -> { + if (ar.succeeded()) { + started.complete(ar.result()); + } else { + started.completeExceptionally(ar.cause()); + } + }); + server = started.get(10, TimeUnit.SECONDS); + port = server.actualPort(); + } + + @AfterEach + void stopServer() throws Exception { + if (vertx != null) { + CompletableFuture closed = new CompletableFuture<>(); + vertx.close(ar -> closed.complete(null)); + closed.get(10, TimeUnit.SECONDS); + } + } + + @Test + @DisplayName("the web UI can still reach the server on the loopback") + void loopbackIsReachable() { + try (Socket socket = new Socket()) { + socket.connect(new InetSocketAddress(InetAddress.getLoopbackAddress(), port), + CONNECT_TIMEOUT_MS); + socket.setSoTimeout(CONNECT_TIMEOUT_MS); + // A whole request and answer, not just a completed handshake: an open port proves the + // socket was accepted, this proves the application behind it still serves. It also lets + // the server finish with the connection before the teardown closes Vert.x under it. + socket.getOutputStream().write("GET / HTTP/1.0\r\n\r\n".getBytes(StandardCharsets.UTF_8)); + socket.getOutputStream().flush(); + String status = new BufferedReader( + new InputStreamReader(socket.getInputStream(), StandardCharsets.UTF_8)).readLine(); + assertNotNull(status, "the server accepted the connection and then answered nothing"); + assertTrue(status.contains("200"), "expected a served response, got: " + status); + } catch (IOException e) { + fail("the application must still be reachable where it serves its own UI: " + e, e); + } + } + + @Test + @DisplayName("no other machine on the network can reach it") + void nonLoopbackIsRefused() { + Optional routable = aNonLoopbackAddressOfThisMachine(); + Assumptions.assumeTrue(routable.isPresent(), + "this machine has no non-loopback IPv4 address, so it cannot set up the situation"); + + InetSocketAddress target = new InetSocketAddress(routable.get(), port); + assertThrows(IOException.class, () -> { + try (Socket socket = new Socket()) { + socket.connect(target, CONNECT_TIMEOUT_MS); + } + }, "the server answered on " + target + ", so it is listening beyond the loopback and the " + + "unauthenticated API is reachable from the local network"); + } + + /** + * An IPv4 address of this machine that another machine could route to, if there is one. + * + *

Loopback, down interfaces and link-local addresses are all skipped: none of them is a + * credible stand-in for "somebody else on the network". + */ + private Optional aNonLoopbackAddressOfThisMachine() { + List candidates = new ArrayList<>(); + try { + for (NetworkInterface itf : Collections.list(NetworkInterface.getNetworkInterfaces())) { + if (!itf.isUp() || itf.isLoopback()) { + continue; + } + for (InetAddress address : Collections.list(itf.getInetAddresses())) { + if (address instanceof Inet4Address + && !address.isLoopbackAddress() + && !address.isLinkLocalAddress()) { + candidates.add(address); + } + } + } + } catch (Exception e) { + return Optional.empty(); + } + return candidates.stream().findFirst(); + } +}