Skip to content

docker: publish the throwaway container on loopback like everything else - #119

Open
otsobide wants to merge 1 commit into
developfrom
feature/bind-docker-run-to-loopback
Open

docker: publish the throwaway container on loopback like everything else#119
otsobide wants to merge 1 commit into
developfrom
feature/bind-docker-run-to-loopback

Conversation

@otsobide

Copy link
Copy Markdown
Owner

The actionable half of #72. It does not close it — the authentication itself
is still absent, and the issue argues against building it before the server is a
release artifact.

The one-line bug hiding in a decision issue

Every path that publishes a port binds loopback — compose (three blocks),
server-aio/Makefile, server-backend/smoke.sh, server-aio/smoke.sh — except
one:

docker/run: docker/build
	docker run --rm -p $(COLLAPSE_PORT):8000 $(IMAGE) $(ARGS)

-p 8000:8000 binds 0.0.0.0. The server's own default is loopback because it
has no authentication, but that cannot help here: the image's ENTRYPOINT pins
--host 0.0.0.0 so a published port reaches it at all, which leaves the port
mapping as the only thing deciding who can connect. On a laptop on a cafe
network, make docker/run put an unauthenticated compression service on the
LAN.

And the advice nobody had written down

threat_model.md and #72 both say "put it behind something that
authenticates". Neither said how, and it turns out the CLI already supports
the ordinary form of it
:

collapse compress notes.txt --server http://user:secret@proxy.internal:8080

Verified against a stub requiring authentication, not assumed:

request header : Authorization: Basic dXNlcjpzZWNyZXQ=
CLI output     : error: the server rejected the request (HTTP 401): authentication required

The rejection is legible because the CLI prefers the server's JSON detail
field.

docs/server.md gains a section for this, and it states the costs in the same
breath — the credential is in shell history and the process list, and in the
desktop's localStorage in clear text if saved as a server — plus what the
client still cannot do: no bearer token, no API key header, no client
certificate. threat_model.md measure 9 links to it instead of repeating the
advice.

Scope

Docs and one Makefile line. No code changes; make test/rust green at 494 as a
sanity check.

`make docker/run` was the one path that bound every interface. Compose, the
all-in-one target and both smoke scripts all publish `127.0.0.1:<port>`; this
one said `-p $(COLLAPSE_PORT):8000`, which is `0.0.0.0`.

The server's own default is loopback precisely because it has no
authentication, but that default cannot help here: the image's ENTRYPOINT pins
`--host 0.0.0.0` so that a published port reaches it at all. The port mapping is
therefore the only thing deciding who can talk to it, and this target handed it
to the local network. On a laptop on a cafe network, one `make docker/run` put
an unauthenticated compression service on the LAN.

Part of #72, which stays open for the authentication itself.

Also documents the advice #72 and `threat_model.md` have both been giving
without ever spelling it out: put it behind something that authenticates. It
turns out **the CLI already works with the ordinary form of that** and nothing
said so. `ureq` sends Basic credentials from a URL's userinfo, so
`--server http://user:secret@proxy:8080` needs no client change.

Verified rather than assumed, against a stub that requires authentication: the
request arrives carrying `Authorization: Basic dXNlcjpzZWNyZXQ=`, and the
rejection comes back as `the server rejected the request (HTTP 401):
authentication required` rather than as a transport failure, because the CLI
prefers the server's JSON `detail`.

The costs are in the same section, because they are the reason someone might
choose differently: the credential lands in shell history and in the process
list, and in the desktop's `localStorage` in clear text if the URL is saved as
a server. And what the client still cannot do — bearer tokens, API keys, client
certificates — is stated so the recipe is not mistaken for more than it is.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant