HTTP server listens on all interfaces without authentication

[lgnap]

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 library
• POST /api/library/download — returns a pack's contents
• POST /api/library/upload — writes a file into the library
• POST /api/library/convert — triggers a conversion
• POST /api/library/remove — deletes a pack
• POST /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 — the listen call, plus a failure
  handler so that an occupied port is reported
• web-ui/src/test/java/studio/webui/ — listen test
• README.md / README_fr.md — document the option if one is added

Implementation plan

1. Replace listen(8080) with listen(port, host), host defaulting to 127.0.0.1.
2. Read address and port from system properties (studio.host, studio.port), following the
   convention already established by studio.open.
3. Pass a handler to listen: log a bind failure explicitly rather than leaving a process
   running that serves nothing.
4. Build the browser URL from the effective port instead of the http://localhost:8080
   constant.
5. Test: the running server answers on 127.0.0.1 and 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 test green on Linux and Windows

Complexity

S — the core fix is one line; most of the work is configurability and the test.

[antoinevalentinHA]

Thanks for this — it was accurate, including the point that CORS is irrelevant here.

Fixed in antoinevalentinHA#41, merged as 59c336c. The server now binds 127.0.0.1.

Two things I found while doing it that are worth recording:

• Remote use was never actually possible. The frontend addresses http://localhost:8080 hardcoded 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.
• HttpServerBindingTest asserts 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 a static 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:

• No coverage measurement, static analysis or contributor guide #5 — the stale CI comment is fixed (docs: correct a CI comment that denied the JavaScript suite antoinevalentinHA/studio#40). The other items there I agree with and have not done.
• Dependencies frozen since 2019-2020, no update visibility #3 — whether CVE-2023-24815 is actually reachable in this configuration is what I plan to look at next. The bind narrows its exposure to the local machine; it does not fix it, and Vert.x is still 3.9.0.
• core module has almost no test coverage #2 — you are right, and it is the module that produces the bytes written to the card. The round-trip framing is the one I would use too.
• Three duplicated conversion paths in LibraryService #4 — agreed on the substance, and the safety net you propose is the right one: the provenance and stream-lifecycle tests must pass unmodified, or the refactor changed behaviour. It is also the riskiest of the five, so it is not next.

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]

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.

• Three duplicated conversion paths in LibraryService #4 → One conversion path in LibraryService instead of six copies of it antoinevalentinHA/studio#47. The duplication turned out to be six copies rather than three: each method branches on the source format. 870 lines to 806. No test added or modified, which is the whole verification — the provenance and stream-lifecycle tests you named pass untouched. Already green on your CI, Windows included.
• core module has almost no test coverage #2 → Round trips and cipher vectors for core, which had one test antoinevalentinHA/studio#48. core from 1 test to 24, suite from 284 to 304. Four KNOWN GAP findings, 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 ..%2F as 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:

the stale CI comment is fixed (antoinevalentinHA#40). The other items there I agree with and have not done.

So one of five is done, and yarn.lock has 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 running mvn package against an unpinned @latest.

That leaves four, unclaimed by either of us:

Coverage measurement	JaCoCo on the Java side, --coverage on 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.
Static analysis	Run SpotBugs once by hand over core and driver, triage what it says, and then decide whether it earns a place in CI. Adding it upfront would drown PRs in inherited warnings.
agent	261 lines, no tests. It does bytecode instrumentation, so that may well be deliberate — but it is not written down anywhere, while TESTING.md documents everything else scrupulously. Either tests or a line saying why not; both answers are fine, silence is not.
CONTRIBUTING.md	How to run both suites, the characterization/specification distinction, the KNOWN GAP convention, the rule that a test changes category deliberately and never silently. Those conventions are real and unusual, and they live in TESTING.md, which is not where a contributor looks.

Plus one thing found since, which belongs in the same family: project.build.sourceEncoding is 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 core gap sit behind a healthy-looking total; CONTRIBUTING.md second, because it is pure writing and someone else can review it; SpotBugs and agent last, 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.