Repository navigation
HTTP server listens on all interfaces without authentication #1
Description
Context
MainVerticle starts the HTTP server without specifying a listen address:
// web-ui/src/main/java/studio/webui/MainVerticle.java:101
vertx.createHttpServer().requestHandler(router).listen(8080);Vert.x's listen(int) overload uses the default host, 0.0.0.0: the server is reachable from
every network interface, not just the loopback.
The API mounted at /api has no authentication, and exposes among others:
GET /api/library/packs— lists the local libraryPOST /api/library/download— returns a pack's contentsPOST /api/library/upload— writes a file into the libraryPOST /api/library/convert— triggers a conversionPOST /api/library/remove— deletes a packPOST /api/device/addFromLibrary,/removeFromDevice,/dump— drive the connected device
On a shared network — public wifi, a corporate or guest network, a shared flat — any machine on
the same segment can therefore read, write and delete in the user's library while STUdio is
running. Since the application is launched by a script that immediately opens a browser, the
user has no reason to suspect anything is listening beyond the loopback.
The CORS filter at MainVerticle.java:67 (CorsHandler.create("http://localhost:.*")) does not
cover this: CORS is enforced by the browser, on requests issued by a web page. It has no effect
on a direct request from curl, a script, or another application.
Description
Bind the server to the loopback only, which matches what the application is: a desktop tool
whose UI is served at http://localhost:8080 — the very URL MainVerticle opens itself, a few
lines after the listen call.
No functionality is lost: the frontend is served by the same server and loaded from localhost.
A related point, to settle here or in a follow-up: port 8080 is hard-coded. It is very often
already taken, and startup then fails with nothing actionable — listen() with no handler
discards the failure silently. Making address and port configurable (via a system property, as
studio.open already is just below) would address both at once.
Affected files
web-ui/src/main/java/studio/webui/MainVerticle.java— thelistencall, plus a failure
handler so that an occupied port is reportedweb-ui/src/test/java/studio/webui/— listen testREADME.md/README_fr.md— document the option if one is added
Implementation plan
- Replace
listen(8080)withlisten(port, host),hostdefaulting to127.0.0.1. - Read address and port from system properties (
studio.host,studio.port), following the
convention already established bystudio.open. - Pass a handler to
listen: log a bind failure explicitly rather than leaving a process
running that serves nothing. - Build the browser URL from the effective port instead of the
http://localhost:8080
constant. - Test: the running server answers on
127.0.0.1and does not answer on the machine's
non-loopback address.
Acceptance criteria
- By default, the server accepts connections on the loopback only
- A failed bind (port in use) produces an explicit error in the logs
- The URL opened in the browser reflects the port actually used
- Listening on another interface remains possible, but as an explicit choice
- Automated test covering loopback accepted / external interface refused
-
mvn -Dskip.installnodeyarn=true -Dskip.yarn=true testgreen on Linux and Windows
Complexity
S — the core fix is one line; most of the work is configurability and the test.
Activity
antoinevalentinHA commentedon Sep 5, 2026
Thanks for this — it was accurate, including the point that CORS is irrelevant here.
Fixed in antoinevalentinHA#41, merged as
59c336c. The server now binds127.0.0.1.Two things I found while doing it that are worth recording:
http://localhost:8080hardcoded throughout — API calls, event bus, even the translation files — so a browser on another machine would receive the page and then send every request to its own loopback. The wide binding exposed the API without ever making the application usable from elsewhere. That settled the design question: the address is fixed, with no override, since a setting would keep the exposure reachable to buy back a capability that does not work.HttpServerBindingTestasserts the consequence rather than the value — a real server on production's address, real sockets, the loopback must answer 200 and a routable address of the same machine must be refused. One trap worth passing on: I first wrote the address as astatic final String, and the red proof failed to reproduce. A compile-time constant is inlined into whatever reads it, so the test held a frozen copy and stayed green while production had been reopened. It is a method now.The hardcoded port you mention at the end is untouched.
Where the rest stands:
If you feel like working on any of these, PRs are very welcome on my fork. #2 and #4 are completely open on my side — I have nothing in progress on either.
lgnap commentedon Sep 5, 2026
Taking you at your word on the division you set out above, and asking about the part it did not cover.
Where the five stand now
#2 and #4 — done, since you said both were completely open.
corefrom 1 test to 24, suite from 284 to 304. FourKNOWN GAPfindings, none of them regressions. The one I would draw your eye to is that an action node option pointing at the first stage node loses the entire option list — options are sector addresses, the list ends at the first zero, and stage nodes start at sector 0. Waiting on a workflow approval.#3 — yours, and you went further than the issue did. You took it, corrected my citation, and reproduced an actual traversal that reading the source would not have found. That produced antoinevalentinHA#42. My opinion on the fix is in the thread on my fork — allowlist rather than denylist, because the set of encodings that survive normalisation is not obvious and a denylist encodes today's set — plus
..%2Fas a case worth pinning even though it 404s now. Nothing for me to do there without walking into what you are already writing.The question: #5
You wrote:
So one of five is done, and
yarn.lockhas since become your antoinevalentinHA#44 — and you took that further than I had, since the nightly build that produces the published archive is the one runningmvn packageagainst an unpinned@latest.That leaves four, unclaimed by either of us:
--coverageon the JS side, published as a CI artifact — and deliberately no failing threshold at first. Measure, look, then decide. A threshold on a project whose coverage is this uneven would just block PRs.coreanddriver, triage what it says, and then decide whether it earns a place in CI. Adding it upfront would drown PRs in inherited warnings.agentTESTING.mddocuments everything else scrupulously. Either tests or a line saying why not; both answers are fine, silence is not.CONTRIBUTING.mdKNOWN GAPconvention, the rule that a test changes category deliberately and never silently. Those conventions are real and unusual, and they live inTESTING.md, which is not where a contributor looks.Plus one thing found since, which belongs in the same family:
project.build.sourceEncodingis not declared, while non-ASCII already appears in a dozen main sources — several of them yours. Compilation uses the platform default and CI passes by luck of the runners. I hit it from the other side and took an em dash back out of a log message rather than add to the bet.Do you want these, or shall we take them?
They are independent of each other, so they can be split rather than assigned as a block. If it helps, my instinct on order is: coverage first, because it is precisely the absence of measurement that let the
coregap sit behind a healthy-looking total;CONTRIBUTING.mdsecond, because it is pure writing and someone else can review it; SpotBugs andagentlast, since both are really "look and decide" rather than "do".Happy to take all four, some, or none — I would rather ask than start on something you were about to write. Same for whether you would prefer these tracked as issues on your repo instead of over here.