Bind the loopback by default, make host and port configurable

[lgnap]

Closes #1.

What this changes

MainVerticle called listen(8080), whose Vert.x overload uses the default host 0.0.0.0. Every interface accepted connections to an API that has no authentication and that lists, downloads, uploads, converts and deletes in the library, and drives a connected device.

Observed on a running instance before the change, not inferred from the source:

$ ss -ltnp | grep 8080
LISTEN 0 4096  *:8080  *:*  users:(("java",pid=15941,...))

$ curl -s http://10.0.50.26:8080/api/library/infos      # LAN address, not localhost
{"path":"/home/<user>/.studio/library/"}

It now listens on 127.0.0.1 unless -Dstudio.host says otherwise, and the port comes from -Dstudio.port, following the convention studio.open already set.

listen() was also called with no handler, so a failed bind was discarded and an occupied port left a running process that served nothing and logged nothing. The bind result is now handled, and the browser opens only once the socket is bound, at the address it was bound to.

The frontend named localhost:8080 in 20 places, which would have made the port configurable in name only. Those are now relative. The event bus is the exception: sockjs-client rejects a URL with no host and no protocol (sockjs.js, "The URL '...' is invalid"), so it derives an absolute URL from window.location.origin. Under yarn start the CRA dev server cannot proxy that connection, so .env.development points it at the Java backend and proxy in package.json forwards the rest.

Attribution

Making the host and port configurable, and deriving the CORS pattern and the browser URL from them, comes from @kairoh's fork — commit 74f53cc, "Configurable listen host and port" (2 April 2022), which predates that fork's move to Quarkus and so applied to the same Vert.x code this fork still runs. Both projects are MPL-2.0.

Credited in three places: a Co-authored-by trailer on the commit, a "Code from other forks" section in the README, and a comment above the code itself.

That commit is credited for what it did and not for more: it keeps listen(port), which still accepts connections on every interface. Binding to the loopback and reporting a failed bind are not from it.

Tests

ServerBindingTest, five cases on the real MainVerticle, deployed with env=dev so the mock story teller is used and libusb is never touched, and with every path pointed at a temporary folder — including both metadata databases, since a missing official database makes the service fetch it over the network.

They pin: the loopback is accepted, every other interface is refused, studio.port is honoured, studio.host can still widen the binding, and a port already in use leaves its occupant alone.

Checked against the previous behaviour rather than assumed — restoring the 0.0.0.0 default makes one fail:

ServerBindingTest$ByDefault.refusesNonLoopback:154
  the server should not be reachable on 10.0.50.26 ==> expected: <false> but was: <true>

Full suite locally on Temurin 11: 287 tests, 0 failures, 39 skipped — 282 before these five. git diff --exit-code clean.

Not verified locally

• Windows. Only Linux was run here. The two cases needing a non-loopback address skip rather than fail on a machine without one.
• The JavaScript suite. It needs Node 12 for react-scripts 3.0.1; this was written on Node 24. App.js, i18n.js and the three services/ files are touched, so packs.test.js, eventBusChannel.test.js and addFromLibrary.test.js want a run.
• The packaged bundle. The PR workflow skips the frontend goals, so a real mvn package has not been exercised with the relative URLs.

Unrelated finding

The project does not declare project.build.sourceEncoding, so compilation uses the platform encoding while non-ASCII already appears in main sources. It works on today's runners by luck of their defaults. Belongs with #5; I kept an ASCII hyphen in the new log message rather than add to the bet.

[antoinevalentinHA]

I merged antoinevalentinHA#41 fifteen minutes after you opened this, and saw it afterwards. Timing, not disregard — this is the better piece of work on several counts.

Three things you found that mine did not, all verified on my master before writing this:

• listen() is called with no result handler, so a failed bind is discarded: an occupied port leaves a process running that serves nothing and logs nothing. Still true on 59c336c.
• The frontend named localhost:8080 in twenty places, which made the port configurable in name only.
• project.build.sourceEncoding is undeclared while non-ASCII already appears in ten-odd main sources — several of which I wrote. Compilation depends on the platform default. You are right that it works by luck of the runners' defaults.

Your ServerBindingTest is also stronger than mine: it deploys the real MainVerticle rather than a server started on production's address.

Your frontend fix invalidates the argument I published on #1. I wrote there that a setting would "buy back a capability that does not work", because the frontend hardcoded localhost. You fixed that, so the premise is gone. I am not going to try to rescue the argument.

The decision still stands, but it now rests on a different and narrower ground: STUdio is a local desktop tool with no authentication at all, so I do not want it to offer a supported way to expose that API on a network. Loopback-only, no override. What would change my mind is authentication, not configurability — if the API is ever safe to expose, studio.host becomes reasonable the same day.

That is a judgement about network architecture, and I would rather say plainly that it is one, not dress it up as the only defensible reading of your PR.

---

On #3, and this is the part that matters most. Your issue is what made me go and check, so credit where it is due — but the result is not what either of us expected.

The strict attribution is wrong: GHSA-53jx-vvf9-4x38 declares >= 4.0.0, < 4.3.8, and the mechanism it describes genuinely does not exist in 3.9.0. The vulnerable branch returns an unsanitised pathParam("*"); 3.9's Utils.pathOffset is a different implementation that never reads it, and StaticHandlerImpl sanitises with removeDots before the call. On the source alone, 3.9.0 looks unaffected.

It is not. removeDots works on / segments, so backslashes survive it, and on Windows the filesystem treats them as separators. I built a probe on the exact production route — a pathless route() with StaticHandler.create() — put a sentinel at the classpath root outside the webroot, and on Windows three of seven request shapes returned it with a 200. An encoded backslash stepping one level up is enough. Two levels and the / variants give 404, so the reach is one level: the classpath root, meaning application resources and whatever sits in the lib/ jars. Not the user's library, not the device, not arbitrary files.

So: your citation is inexact, your instinct was right, and the behaviour is confirmed by reproduction rather than by reading. I would have concluded "out of range, probably fine" from the source. The test is what settled it.

Two consequences I want to state rather than leave implied:

• The loopback bind is not a fix for this. It narrows exposure to the local machine on master; the defect is intact and still exploitable there.
• The published 0.4.3-fork.1 artifact predates that bind, listens on every interface, and is therefore reachable from a local network. That is a separate matter I have to deal with.

This deserves its own correction, independent of the binding. My inclination is something small and targeted — refusing paths carrying a backslash or its encodings before they reach StaticHandler, with a Windows test reproducing exactly the requests that read the sentinel — rather than an improvised move to Vert.x 4.x. I am not claiming that is the only correct answer, and if you see a better one I would rather hear it before I write anything.

---

If you want this integrated: rebase on 59c336c and open it against my repo. It will conflict — antoinevalentinHA#40 and antoinevalentinHA#41 landed after your base — and ServerBindingTest and HttpServerBindingTest overlap, with yours being the one I would keep. The host and port configurability is the part I would ask you to drop, for the reason above. The rest looks worth bringing over, and I'd be happy to review it on that basis.

[lgnap]

Thanks — and the correction on the CVE is taken. I cited an advisory without checking whether its mechanism existed in 3.9.0, and it does not. "Out of range, probably fine" was the reading the source supported; you got the real answer by building a probe. I will cite ranges more carefully.

Opinion only on this one. I am not asking to write the fix — you have the reproduction and the Windows machine, and splitting that across two people would be worse. What follows is review, and one place where I think your plan needs a small change.

I reproduced the mechanism independently, and it is slightly worse than "backslashes survive"

Calling io.vertx.core.http.impl.HttpUtils.normalizePath from vertx-core 3.9.0 directly, no HTTP server involved:

/webroot/../secret      ->  /secret            "..' over '/' is collapsed
/..\secret              ->  /..\secret         backslash survives, and the ".." is not collapsed
/%2e%2e%5csecret        ->  /..%5csecret       %2e IS decoded, %5c is NOT
/..%2Fsecret            ->  /..%2Fsecret       %2F is not decoded either
/%252e%252e%255csecret  ->  /%252e%252e%255csecret

So the normaliser is asymmetric about percent-encoding: it decodes %2e into . and then collapses on /, but leaves %5c and %2F encoded. That is the part I would build the fix around, because it decides where the check has to sit.

Where I would differ from your plan

You proposed refusing paths carrying a backslash or its encodings before StaticHandler. I agree with the shape — targeted, testable, no improvised jump to 4.x — but I would invert it from a denylist to an allowlist, for one concrete reason: a denylist has to enumerate exactly which encodings survive normalisation, and the table above shows that set is not obvious. %5c survives, %2e does not, %2F survives. Any list you write is a list of the encodings this version happens to leave alone; a Vert.x bump could change which ones reach you, and the check would silently stop matching while still passing its tests.

What I would assert instead, on the normalised path, before it reaches StaticHandler:

• it starts with /
• every remaining character is in a small allowed set — letters, digits, ., -, _, /
• no segment equals ..

Anything else is refused. That rejects \, %5c, %2F, %25, and whatever the next encoding turns out to be, without needing to know about it in advance. The web UI is a bundled React build: its asset names are hashed and boring, so the allowlist costs nothing in practice. If a real asset name ever fails it, that is a signal worth having rather than a bug.

One case worth adding to your Windows probe either way: ..%2F. It survives normalisation identically to ..%5c, so if anything downstream ever decodes %2F, the same class of bug becomes cross-platform rather than Windows-only. I would expect it to 404 today — your note that "the / variants give 404" suggests you already saw that — but pinning it as a test means a future decode change goes red instead of quiet.

On the two consequences you stated

Both correct, and I want to underline the second. The loopback bind is not a fix, agreed — it narrows who can reach the defect, not the defect.

The published 0.4.3-fork.1 is the one that matters, because it is the combination that is dangerous: it predates the bind, so it listens on every interface, and it carries the traversal. Either alone is much less serious than both together. I have that artifact on disk, which is how this whole thread started.

That is a judgement call about disclosure that is yours, not mine, so I will only say what I would weigh: it is a pre-release, the README says plainly that it is work in progress, and the reach you measured is application resources and lib/ jars rather than the user's library or the device. Against that, anyone who downloaded it and runs it on a shared network is exposed today. Pulling or annotating the release seems proportionate; a full advisory for a one-level read of bundled resources probably does not. You are better placed than I am to judge how many people are running it.

Not a fix, but worth knowing

project.build.sourceEncoding is still undeclared, and I hit it from the other side: I put an em dash in a log message, then took it out again, because compilation uses the platform default and CI passes by luck of the runners. Unrelated to the traversal, but it is in the same "works today for reasons nobody chose" family. It is in #5.

Happy to review whatever you write, and to test it on Linux — where I can at least confirm the allowlist does not break any real asset path in the bundle. The Windows half only you can do.