[ChiragAgg5k]

Port 80 is hard-coded, so the server can't run as a non-root user where low ports are privileged (ip_unprivileged_port_start=1024).

port: (string) System::getEnv('GEO_PORT', '80'),

The default stays 80. The image HEALTHCHECK reads the same variable.

Tested with a local build, uid 82, --cap-drop ALL, no-new-privileges, ip_unprivileged_port_start=1024:

GEO_PORT	Result
unset	bind fails, as on the released image
8080	/v1/health 200, /v1/ips/8.8.8.8 returns the US record

The E2E job covers both: the default through compose, and GEO_PORT=8081 as uid 82 with all capabilities dropped.

[hansi-codes]

🟢 Tier S · Ready to merge

The Geo server now reads its listener port from GEO_PORT, while retaining port 80 as the default, and the Docker health check probes that configured port. The CI workflow also exercises a non-root container listening on a non-default port.

Latest changes: The new CI step starts the image as UID 82 with all capabilities dropped and GEO_PORT=8081, then checks its health endpoint on port 8081.

Verdict	New comments	Fixed	Still open
✅ Approved	0	1	0

📂 Walkthrough · 3

File	Change
src/Geo/Server/Server.php	Configures the server listener from GEO_PORT, defaulting to port 80.
Dockerfile	Makes the image health check probe the port specified by GEO_PORT, defaulting to port 80.
.github/workflows/tests.yml	Adds a CI check that verifies the server responds on a non-default port when run as a non-root user without capabilities.

✅ Fixed since the last review · 1

• Cover the non-default listener port in E2E tests · src/Geo/Server/Server.php:42

---

_{Reviewed the commits since fa8743c · Details · Comment @hansi-codes review to re-run, or mention @hansi-codes with a question.}

[hansi-codes]

🔵 Tier A · Looks good to merge. Summary

[hansi-codes]

🟢 Tier S · Looks good to merge. Summary