CI: Apply Go linter recommendations to "internal/server" package #5330

Signed-off-by: Michael Mayer <michael@photoprism.app>

internal/server/README.md

@@ -0,0 +1,61 @@
+## PhotoPrism — HTTP Server
+
+**Last Updated:** November 22, 2025
+
+### Overview
+
+`internal/server` wires Gin, middleware, and configuration into the PhotoPrism HTTP/HTTPS/WebDAV servers. It owns startup/shutdown orchestration, route registration, and helpers for recovery/logging. Subpackages (`process`, `limits`, etc.) are kept lightweight so CLI commands and workers can embed the same server behavior without duplicating boilerplate.
+
+#### Context & Constraints
+
+- Uses the configured `config.Config` to decide TLS, AutoTLS, Unix sockets, proxies, compression, and trusted headers.
+- Middleware must stay small and deterministic because it runs on every request; heavy logic belongs in handlers.
+- Panics are recovered by `Recovery()` which logs stack traces and returns 500.
+- Startup supports mutually exclusive endpoints: Unix socket, HTTPS with certs, AutoTLS (with redirect listener), or plain HTTP.
+
+#### Goals
+
+- Provide a single entrypoint (`Start`) that configures listeners, middleware, and routes consistently.
+- Keep health/readiness endpoints lightweight and cache-safe.
+- Ensure redirect and TLS listeners include sensible timeouts.
+
+#### Non-Goals
+
+- Managing Docker/Traefik lifecycle (handled by compose files).
+- Serving static files directly; templates are loaded via Gin and routed by `routes_webapp.go`.
+
+### Package Layout (Code Map)
+
+- `start.go` — main startup flow, listener selection (HTTP/HTTPS/AutoTLS/Unix socket), graceful shutdown.
+- `routes_webapp.go` — Web UI routes and shared method helpers (`MethodsGetHead`).
+- `recovery.go` — panic recovery middleware with stack trace logging.
+- `logger.go` — request logging middleware (enabled in debug mode).
+- `security.go` — security headers and trusted proxy/platform handling.
+- `webdav_*.go` & tests — WebDAV handlers and regression tests for overwrite, traversal, and metadata flags.
+- `process/` — light wrappers for server process metadata.
+
+### Related Packages
+
+- `internal/api` — registers REST endpoints consumed by `registerRoutes`.
+- `internal/config` — supplies HTTP/TLS/socket settings, compression, proxies, and base URI paths.
+- `internal/server/process` — exposes process ID for logging.
+- `pkg/http/header` — shared HTTP header constants used by health endpoints.
+
+### Configuration & Safety Notes
+
+- Compression: only gzip is enabled; brotli requests log a notice.
+- Trusted proxies/platform headers are read from config; misconfiguration may expose client IP spoofing—keep the list tight.
+- AutoTLS: uses `autocert` and spins up a redirect listener with explicit read/write timeouts; ensure ports 80/443 are reachable.
+- Unix sockets: optional `force` query removes stale sockets; permissions can be set via `mode` query.
+- Health endpoints (`/livez`, `/health`, `/healthz`, `/readyz`) return `Cache-Control: no-store` and `Access-Control-Allow-Origin: *`.
+
+### Testing
+
+- Lint & unit tests: `golangci-lint run ./internal/server...` and `go test ./internal/server/...`
+- WebDAV behaviors are covered by `webdav_*_test.go`; they rely on temp directories and in-memory routers.
+
+### Operational Tips
+
+- Prefer `Start` with context cancellation so graceful shutdown is triggered (`server.Close()`).
+- When adding routes, register them in `registerRoutes` and reuse `MethodsGetHead` for safe verbs.
+- Keep middleware light; log or enforce security at the edge (Traefik) when possible, but maintain server-side defaults for defense in depth.

internal/server/recovery.go

@@ -48,6 +48,7 @@ func stack(skip int) []byte {
 		// Print this much at least.  If we cannot find the source, it won't show.
 		fmt.Fprintf(buf, "%s:%d (0x%x)\n", file, line, pc)
 		if file != lastFile {
+			// #nosec G304 -- file path comes from runtime.Caller and is limited to source files.
 			data, err := os.ReadFile(file)
 			if err != nil {
 				continue
@@ -90,6 +91,6 @@ func function(pc uintptr) []byte {
 	if period := bytes.Index(name, dot); period >= 0 {
 		name = name[period+1:]
 	}
-	name = bytes.Replace(name, centerDot, dot, -1)
+	name = bytes.ReplaceAll(name, centerDot, dot)
 	return name
 }

internal/server/routes_webapp.go

@@ -13,6 +13,7 @@ import (
 	"github.com/photoprism/photoprism/pkg/i18n"
 )
 
+// MethodsGetHead enumerates the safe GET/HEAD methods used by web app routes.
 var MethodsGetHead = []string{http.MethodGet, http.MethodHead}
 
 // registerWebAppRoutes adds routes for the web user interface.

internal/server/start.go

@@ -41,7 +41,7 @@ func Start(ctx context.Context, conf *config.Config) {
 	// Set web server mode.
 	if conf.HttpMode() != "" {
 		gin.SetMode(conf.HttpMode())
-	} else if conf.Debug() == false {
+	} else if !conf.Debug() {
 		gin.SetMode(gin.ReleaseMode)
 	}
 
@@ -154,7 +154,7 @@ func Start(ctx context.Context, conf *config.Config) {
 
 		// Check if the Unix socket already exists and delete it if the force flag is set.
 		if fs.SocketExists(unixSocket.Path) {
-			if txt.Bool(unixSocket.Query().Get("force")) == false {
+			if !txt.Bool(unixSocket.Query().Get("force")) {
 				Fail("server: %s socket %s already exists", clean.Log(unixSocket.Scheme), clean.Log(unixSocket.Path))
 				return
 			} else if removeErr := os.Remove(unixSocket.Path); removeErr != nil {
@@ -278,7 +278,15 @@ func StartAutoTLS(s *http.Server, m *autocert.Manager, conf *config.Config) {
 	var g errgroup.Group
 
 	g.Go(func() error {
-		return http.ListenAndServe(fmt.Sprintf("%s:%d", conf.HttpHost(), conf.HttpPort()), m.HTTPHandler(http.HandlerFunc(redirect)))
+		redirectSrv := &http.Server{
+			Addr:              fmt.Sprintf("%s:%d", conf.HttpHost(), conf.HttpPort()),
+			Handler:           m.HTTPHandler(http.HandlerFunc(redirect)),
+			ReadHeaderTimeout: time.Minute,
+			ReadTimeout:       5 * time.Second,
+			WriteTimeout:      10 * time.Second,
+		}
+
+		return redirectSrv.ListenAndServe()
 	})
 
 	g.Go(func() error {

internal/server/webdav_actions_test.go

@@ -18,8 +18,10 @@ func TestWebDAVSetFavoriteFlag_CreatesYamlOnce(t *testing.T) {
 	yml := filepath.Join(filepath.Dir(file), "img.yml")
 	assert.FileExists(t, yml)
 	// Write a marker and ensure second call doesn't overwrite content
+	// #nosec G304 -- test reads file created in a temp directory.
 	orig, _ := os.ReadFile(yml)
 	WebDAVSetFavoriteFlag(file)
+	// #nosec G304 -- test reads file created in a temp directory.
 	now, _ := os.ReadFile(yml)
 	assert.Equal(t, string(orig), string(now))
 }

internal/server/webdav_write_test.go

@@ -58,6 +58,7 @@ func TestWebDAVWrite_MKCOL_PUT(t *testing.T) {
 	assert.InDelta(t, 201, w.Code, 1)
 	// file exists
 	path := filepath.Join(conf.OriginalsPath(), "wdvdir", "hello.txt")
+	// #nosec G304 -- test reads file created under controlled temp directory.
 	b, err := os.ReadFile(path)
 	assert.NoError(t, err)
 	assert.Equal(t, "hello", string(b))
@@ -147,7 +148,7 @@ func TestWebDAVWrite_OverwriteSemantics(t *testing.T) {
 	authBasic(req)
 	r.ServeHTTP(w, req)
 	// Success (201/204 acceptable)
-	if !(w.Code == 201 || w.Code == 204) {
+	if w.Code != http.StatusCreated && w.Code != http.StatusNoContent {
 		t.Fatalf("expected success for Overwrite=T, got %d", w.Code)
 	}
 	b, _ = os.ReadFile(filepath.Join(conf.OriginalsPath(), "dst", "f.txt"))
@@ -172,7 +173,7 @@ func TestWebDAVWrite_OverwriteSemantics(t *testing.T) {
 	req.Header.Set("Overwrite", "T")
 	authBasic(req)
 	r.ServeHTTP(w, req)
-	if !(w.Code == 201 || w.Code == 204) {
+	if w.Code != http.StatusCreated && w.Code != http.StatusNoContent {
 		t.Fatalf("expected success for MOVE Overwrite=T, got %d", w.Code)
 	}
 	assert.NoFileExists(t, filepath.Join(conf.OriginalsPath(), "src", "g.txt"))
@@ -196,7 +197,7 @@ func TestWebDAVWrite_MoveMissingDestination(t *testing.T) {
 	authBasic(req)
 	r.ServeHTTP(w, req)
 	// Expect failure (not 201/204)
-	if w.Code == 201 || w.Code == 204 {
+	if w.Code == http.StatusCreated || w.Code == http.StatusNoContent {
 		t.Fatalf("expected failure when Destination header missing, got %d", w.Code)
 	}
 	// Source remains
@@ -220,7 +221,7 @@ func TestWebDAVWrite_CopyInvalidDestinationPrefix(t *testing.T) {
 	authBasic(req)
 	r.ServeHTTP(w, req)
 	// Expect failure
-	if w.Code == 201 || w.Code == 204 {
+	if w.Code == http.StatusCreated || w.Code == http.StatusNoContent {
 		t.Fatalf("expected failure for invalid Destination prefix, got %d", w.Code)
 	}
 	// Destination not created
@@ -242,7 +243,7 @@ func TestWebDAVWrite_MoveNonExistentSource(t *testing.T) {
 	authBasic(req)
 	r.ServeHTTP(w, req)
 	// Expect failure (e.g., 404)
-	if w.Code == 201 || w.Code == 204 {
+	if w.Code == http.StatusCreated || w.Code == http.StatusNoContent {
 		t.Fatalf("expected failure moving non-existent source, got %d", w.Code)
 	}
 	assert.NoFileExists(t, filepath.Join(conf.OriginalsPath(), "dst2", "file.txt"))
@@ -270,7 +271,7 @@ func TestWebDAVWrite_CopyTraversalDestination(t *testing.T) {
 	authBasic(req)
 	r.ServeHTTP(w, req)
 	// Expect success with sanitized destination inside base
-	if !(w.Code == 201 || w.Code == 204) {
+	if w.Code != http.StatusCreated && w.Code != http.StatusNoContent {
 		t.Fatalf("expected success (sanitized), got %d", w.Code)
 	}
 	// Not created above originals; created as /originals/evil.txt
@@ -300,7 +301,7 @@ func TestWebDAVWrite_MoveTraversalDestination(t *testing.T) {
 	req.Header.Set("Destination", conf.BaseUri(WebDAVOriginals)+"/../evil2.txt")
 	authBasic(req)
 	r.ServeHTTP(w, req)
-	if !(w.Code == 201 || w.Code == 204) {
+	if w.Code != http.StatusCreated && w.Code != http.StatusNoContent {
 		t.Fatalf("expected success (sanitized) for MOVE, got %d", w.Code)
 	}
 	// Source removed; destination created inside base, not outside