The plaintext app port binds all interfaces and the README has no reverse-proxy deployment section #268
Reference in New Issue
Block a user
Delete Branch "%!s()"
Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?
Found by a TLS deployment audit that stood up real nginx terminating TLSv1.3 in front of the app — the configuration the owner will actually deploy, and one nothing had previously exercised.
The defect
internal/server/http.go:28bindsfmt.Sprintf(":%d", Port). There is no bind-address configuration, so the plaintext app listens on every interface and is reachable directly, bypassing the TLS proxy entirely.Verified:
ss -ltnshowsLISTEN *:19001, and from a separate hostcurl http://172.17.0.1:19001/pages/loginreturns 200 — the full admin UI in cleartext. The unauthenticated receiver is reachable the same way.On an internet-facing host this is the difference between "a TLS-only service" and "a TLS service with a cleartext twin on another port".
Why the README is the larger half
A scripted scan of every fenced code block in
README.mdforproxy_pass|server_name|listen|reverse_proxy|proxy_set_header|ssl_certificatereturns zero matches. There is no reverse-proxy deployment section at all, so the deployment shape this product is built for has nothing copy-pasteable, and nothing warns the operator to bind or firewall the app port.Four further omissions the audit proved matter, all in the same missing section:
Hostheader requirement is never stated. Not theoretical: the audit's first correct-looking nginx config used the widely-copiedproxy_set_header Host $host, which strips the port, and login failed with403 origin invalid.$http_hostfixed it. Any operator on a non-443 port hits this with nothing to search for.client_max_body_sizeis never mentioned. nginx's default happens to equal the app's 1 MB cap, so nothing breaks today — but an operator raising webhooker's limit will silently keep hitting nginx's, and the 413 becomes nginx's HTML page instead of the app's message.What the README already gets right, verified by execution: the
TRUSTED_PROXIESsection is accurate in every particular tested — right-to-left chain walk, untrusted-peer fallback,X-Real-IPandTrue-Client-IPnever read, unparseable value aborts startup, empty value warns. The prod/devSecuretable is accurate. This issue is about what is missing, not what is wrong.Definition of done
README.mdwith a complete, correct, copy-pasteable nginx server block:ssl_certificate,proxy_set_header Host $http_host(NOT$host),X-Forwarded-For $proxy_add_x_forwarded_for,X-Forwarded-Proto $scheme,client_max_body_size.WEBHOOKER_ENVIRONMENT=prodor lose session-cookieSecure(#269); setTRUSTED_PROXIESor per-IP rate limiting collapses into one global bucket; log the client IP at the proxy because webhooker's own log records only the proxy.make fmtrun over the changed markdown.Verification
make checkgreen.Plan.
Config. New
BIND_ADDRESS, parsed withnetip.ParseAddr— IP literal only, no hostnames, so there is no DNS lookup at startup and no ambiguity about which of several resolved addresses gets bound. Set-but-unparseable (garbage,localhost,10.0.0.1:8080) aborts startup naming the variable and the value, per the existing iron rule. An address that parses but is not on the host fails atListenAndServeand exits non-zero through the existingshutdownOnListenFailurepath.internal/server/http.goswitches fromfmt.Sprintf(":%d")tonet.JoinHostPort.Default:
127.0.0.1. This is the judgement call, so stating it up front. Loopback is the only default that does not silently publish the admin UI and the unauthenticated receiver in cleartext on every interface, which is the defect. It does break a container, where a loopback-bound process is unreachable even with-p— so the documenteddocker rungains-e BIND_ADDRESS=0.0.0.0and the README calls out that containers must set it. That breakage is loud (the container's healthcheck fails immediately) rather than silent, which is the right direction for a security default; the alternative default fails silently in the direction of exposure. No container auto-detection — that would be magic that is wrong in the cases that matter.README. New "Deployment behind a reverse proxy" section with a working nginx
serverblock (ssl_certificate,proxy_set_header Host $http_host— not$host—X-Forwarded-For $proxy_add_x_forwarded_for,X-Forwarded-Proto $scheme,client_max_body_size,proxy_passto a literal127.0.0.1rather thanlocalhostso IPv6 resolution cannot miss the v4 loopback bind), plus the four operator requirements the audit proved matter: bind or firewall the app port,WEBHOOKER_ENVIRONMENT=prod,TRUSTED_PROXIES, and logging the client IP at the proxy. One sentence on the fixed HSTS value. The existingTRUSTED_PROXIESandSecure-table sections are correct and are not being rewritten — the new section links to them.Verification.
make check; real nginx terminating TLS with a self-signed cert, driven from the new section verbatim, proving login, delivery and the event log over HTTPS; the app port unreachable off-host with a positive control through the proxy; the documenteddocker runstill working.Done in #277.
Correction to the plan above. I claimed a container without
BIND_ADDRESSwould fail loudly via its health check. I then measured it instead of assuming, and the mechanism is different from what I described — but the conclusion holds, more strongly. The health check requestshttp://localhost:8080; busyboxwgettries::1first, a127.0.0.1bind is not there, and the container goesunhealthyafter ~95s withConnection refusedin its health log. Not "passes anyway", and not "fails immediately" — it fails visibly, on the health-check clock. The README states this, and it independently confirms the section's instruction toproxy_passat a literal127.0.0.1rather thanlocalhost.BIND_ADDRESSdefaults to127.0.0.1, IP literals only.internal/serverbuilds the listen address withnet.JoinHostPort.Verified by execution, not by inspection:
POSTto the receiver, outbound delivery to a target, and the event log all worked over HTTPS on a non-443 port.$hostclaim reproduced: withHost $hostthe loginPOSTis403and the app logsreason="origin invalid"; withHost $http_hostit is303. One character.172.17.0.1): the app port refuses the connection under the default, while the proxy port answers200. Same binary withBIND_ADDRESS=0.0.0.0showsLISTEN *:19160and serves that container200— the reported defect reproduced, then withheld by the default.not-an-address,localhost,127.0.0.1:8080each exit 1 from config;10.99.99.99parses and exits 1 at listen withcannot assign requested address.docker run, with-e BIND_ADDRESS=0.0.0.0added and-pnarrowed to a host address, comes uphealthyand serves the published port.make checkgreen: 20 packages, no data race, lint0 issues.in Docker.One disclosed fix beyond the issue:
http.Serverwas assigned by the serving goroutine and read by the fx stop hook with nothing ordering them — a data race the detector reports as soon as anything starts and stops the server, and a nil dereference if a stop lands first. It is now constructed inNew. No test could start and stop the server without it. Details in the PR.