nginx: PORT env override is documented but does not exist; add it and server_tokens off #26

Open
opened 2026-08-09 03:44:34 +02:00 by clawbot · 0 comments
Collaborator

Problem

1. PORT is not overridable

REPO_POLICIES.md: "Dockerized web services listen on port 8080 by default, overridable with PORT."

nginx.conf:2 is a static literal:

listen 8080;

There is no envsubst, no templates/ directory, no entrypoint script. Dockerfile:16 copies the file verbatim to /etc/nginx/conf.d/netwatch.conf. The override simply does not exist.

README.md:138 claims otherwise — "Listens on port 8080 by default (override with PORT env var)" — so the documentation is wrong. TODO.md:31 confirms the hardcoding was deliberate at the time: "port hardcoded to 8080". (The README correction is tracked in #24; this issue is the code fix.)

Note the backend already does this correctly: backend/internal/config/config.go defaults PORT to 8080 via viper and it is genuinely overridable. Only the nginx frontend image is non-compliant.

2. server_tokens off; is absent

nginx advertises its exact version in every response header and on every error page. REPO_POLICIES.md: "if a standard security hardening measure exists for HTTP services and is not listed here, it is still expected."

3. Minor, same file

  • No location = /50x.html error page mapping.
  • No gzip. The SPA ships ~28 kB of JS and ~18 kB of CSS uncompressed.

Definition of done

  • The listen port is driven by the PORT environment variable, defaulting to 8080 when unset. The official nginx image supports /etc/nginx/templates/*.conf.template with envsubst — prefer that over a hand-rolled entrypoint script.
  • Verify both paths: build and run the image with no PORT set and confirm it listens on 8080; run it with PORT=9090 and confirm it listens on 9090. Report both results in the PR.
  • Only the intended variables are substituted. A bare envsubst over an nginx config will eat $uri, $host, and every other nginx runtime variable — the template must restrict substitution to ${PORT} explicitly. This is the most likely way to get this wrong; confirm in the PR that try_files $uri $uri/ /index.html survives intact in the rendered config.
  • server_tokens off; is set.
  • EXPOSE in the Dockerfile still documents the default of 8080.
  • docker build . succeeds; make check passes.
  • TODO.md updated in the same commit.
  • Commit title ends with (closes #N).

Implementation requirements

  • Coordinate with #18, which also modifies nginx.conf to add security headers. Whichever lands second must rebase onto the first rather than reverting it. If both are in flight, say so in the PR description.
  • Do not change the existing set_real_ip_from reverse-proxy configuration or the stdout access logging — both are already correct.
  • Do not correct README.md:138 here; that belongs to #24. Fixing the code is this issue's job.
  • Gzip and the error page mapping are optional in this commit — include them only if they do not complicate the port work. If you skip them, note it.
  • No attribution trailers in the commit message.
## Problem ### 1. `PORT` is not overridable `REPO_POLICIES.md`: "Dockerized web services listen on port 8080 by default, **overridable with `PORT`**." `nginx.conf:2` is a static literal: ```nginx listen 8080; ``` There is no `envsubst`, no `templates/` directory, no entrypoint script. `Dockerfile:16` copies the file verbatim to `/etc/nginx/conf.d/netwatch.conf`. The override simply does not exist. `README.md:138` claims otherwise — "Listens on port 8080 by default (override with `PORT` env var)" — so the documentation is wrong. `TODO.md:31` confirms the hardcoding was deliberate at the time: "port hardcoded to 8080". (The README correction is tracked in #24; **this** issue is the code fix.) Note the backend already does this correctly: `backend/internal/config/config.go` defaults `PORT` to 8080 via viper and it is genuinely overridable. Only the nginx frontend image is non-compliant. ### 2. `server_tokens off;` is absent nginx advertises its exact version in every response header and on every error page. `REPO_POLICIES.md`: "if a standard security hardening measure exists for HTTP services and is not listed here, it is still expected." ### 3. Minor, same file - No `location = /50x.html` error page mapping. - No gzip. The SPA ships ~28 kB of JS and ~18 kB of CSS uncompressed. ## Definition of done - [ ] The listen port is driven by the `PORT` environment variable, defaulting to `8080` when unset. The official nginx image supports `/etc/nginx/templates/*.conf.template` with `envsubst` — prefer that over a hand-rolled entrypoint script. - [ ] Verify both paths: build and run the image with no `PORT` set and confirm it listens on 8080; run it with `PORT=9090` and confirm it listens on 9090. Report both results in the PR. - [ ] Only the intended variables are substituted. A bare `envsubst` over an nginx config will eat `$uri`, `$host`, and every other nginx runtime variable — the template must restrict substitution to `${PORT}` explicitly. This is the most likely way to get this wrong; confirm in the PR that `try_files $uri $uri/ /index.html` survives intact in the rendered config. - [ ] `server_tokens off;` is set. - [ ] `EXPOSE` in the `Dockerfile` still documents the default of 8080. - [ ] `docker build .` succeeds; `make check` passes. - [ ] `TODO.md` updated in the same commit. - [ ] Commit title ends with ` (closes #N)`. ## Implementation requirements - Coordinate with #18, which also modifies `nginx.conf` to add security headers. Whichever lands second must rebase onto the first rather than reverting it. If both are in flight, say so in the PR description. - Do not change the existing `set_real_ip_from` reverse-proxy configuration or the stdout access logging — both are already correct. - Do not correct `README.md:138` here; that belongs to #24. Fixing the code is this issue's job. - Gzip and the error page mapping are optional in this commit — include them only if they do not complicate the port work. If you skip them, note it. - No attribution trailers in the commit message.
clawbot added this to the 1.0.0 milestone 2026-08-09 03:44:34 +02:00
Sign in to join this conversation.
1 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: sneak/netwatch#26