diff --git a/internal/handlers/app.go b/internal/handlers/app.go index 46d957e..596337d 100644 --- a/internal/handlers/app.go +++ b/internal/handlers/app.go @@ -32,11 +32,7 @@ func (h *Handlers) HandleAppNew() http.HandlerFunc { return func(writer http.ResponseWriter, request *http.Request) { data := h.addGlobals(map[string]any{}, request) - err := tmpl.ExecuteTemplate(writer, "app_new.html", data) - if err != nil { - h.log.Error("template execution failed", "error", err) - http.Error(writer, "Internal Server Error", http.StatusInternalServerError) - } + h.renderTemplate(writer, tmpl, "app_new.html", data) } } @@ -66,7 +62,7 @@ func (h *Handlers) HandleAppCreate() http.HandlerFunc { //nolint:funlen // valid if name == "" || repoURL == "" { data["Error"] = "Name and repository URL are required" - _ = tmpl.ExecuteTemplate(writer, "app_new.html", data) + h.renderTemplate(writer, tmpl, "app_new.html", data) return } @@ -99,7 +95,7 @@ func (h *Handlers) HandleAppCreate() http.HandlerFunc { //nolint:funlen // valid if createErr != nil { h.log.Error("failed to create app", "error", createErr) data["Error"] = "Failed to create app: " + createErr.Error() - _ = tmpl.ExecuteTemplate(writer, "app_new.html", data) + h.renderTemplate(writer, tmpl, "app_new.html", data) return } @@ -160,11 +156,7 @@ func (h *Handlers) HandleAppDetail() http.HandlerFunc { "Success": request.URL.Query().Get("success"), }, request) - err := tmpl.ExecuteTemplate(writer, "app_detail.html", data) - if err != nil { - h.log.Error("template execution failed", "error", err) - http.Error(writer, "Internal Server Error", http.StatusInternalServerError) - } + h.renderTemplate(writer, tmpl, "app_detail.html", data) } } @@ -193,11 +185,7 @@ func (h *Handlers) HandleAppEdit() http.HandlerFunc { "App": application, }, request) - err := tmpl.ExecuteTemplate(writer, "app_edit.html", data) - if err != nil { - h.log.Error("template execution failed", "error", err) - http.Error(writer, "Internal Server Error", http.StatusInternalServerError) - } + h.renderTemplate(writer, tmpl, "app_edit.html", data) } } @@ -266,7 +254,7 @@ func (h *Handlers) HandleAppUpdate() http.HandlerFunc { //nolint:funlen // valid "App": application, "Error": "Failed to update app", }, request) - _ = tmpl.ExecuteTemplate(writer, "app_edit.html", data) + h.renderTemplate(writer, tmpl, "app_edit.html", data) return } @@ -390,11 +378,7 @@ func (h *Handlers) HandleAppDeployments() http.HandlerFunc { "Deployments": deployments, }, request) - err := tmpl.ExecuteTemplate(writer, "deployments.html", data) - if err != nil { - h.log.Error("template execution failed", "error", err) - http.Error(writer, "Internal Server Error", http.StatusInternalServerError) - } + h.renderTemplate(writer, tmpl, "deployments.html", data) } } diff --git a/internal/handlers/auth.go b/internal/handlers/auth.go index b88e81c..5a719cb 100644 --- a/internal/handlers/auth.go +++ b/internal/handlers/auth.go @@ -13,11 +13,7 @@ func (h *Handlers) HandleLoginGET() http.HandlerFunc { return func(writer http.ResponseWriter, request *http.Request) { data := h.addGlobals(map[string]any{}, request) - err := tmpl.ExecuteTemplate(writer, "login.html", data) - if err != nil { - h.log.Error("template execution failed", "error", err) - http.Error(writer, "Internal Server Error", http.StatusInternalServerError) - } + h.renderTemplate(writer, tmpl, "login.html", data) } } @@ -42,7 +38,7 @@ func (h *Handlers) HandleLoginPOST() http.HandlerFunc { if username == "" || password == "" { data["Error"] = "Username and password are required" - _ = tmpl.ExecuteTemplate(writer, "login.html", data) + h.renderTemplate(writer, tmpl, "login.html", data) return } @@ -50,7 +46,7 @@ func (h *Handlers) HandleLoginPOST() http.HandlerFunc { user, authErr := h.auth.Authenticate(request.Context(), username, password) if authErr != nil { data["Error"] = "Invalid username or password" - _ = tmpl.ExecuteTemplate(writer, "login.html", data) + h.renderTemplate(writer, tmpl, "login.html", data) return } @@ -60,7 +56,7 @@ func (h *Handlers) HandleLoginPOST() http.HandlerFunc { h.log.Error("failed to create session", "error", sessionErr) data["Error"] = "Failed to create session" - _ = tmpl.ExecuteTemplate(writer, "login.html", data) + h.renderTemplate(writer, tmpl, "login.html", data) return } diff --git a/internal/handlers/dashboard.go b/internal/handlers/dashboard.go index a85c4da..caadf06 100644 --- a/internal/handlers/dashboard.go +++ b/internal/handlers/dashboard.go @@ -69,10 +69,6 @@ func (h *Handlers) HandleDashboard() http.HandlerFunc { "AppStats": appStats, }, request) - execErr := tmpl.ExecuteTemplate(writer, "dashboard.html", data) - if execErr != nil { - h.log.Error("template execution failed", "error", execErr) - http.Error(writer, "Internal Server Error", http.StatusInternalServerError) - } + h.renderTemplate(writer, tmpl, "dashboard.html", data) } } diff --git a/internal/handlers/handlers.go b/internal/handlers/handlers.go index 40a890a..9146fd0 100644 --- a/internal/handlers/handlers.go +++ b/internal/handlers/handlers.go @@ -2,6 +2,7 @@ package handlers import ( + "bytes" "encoding/json" "log/slog" "net/http" @@ -18,6 +19,7 @@ import ( "git.eeqj.de/sneak/upaas/internal/service/auth" "git.eeqj.de/sneak/upaas/internal/service/deploy" "git.eeqj.de/sneak/upaas/internal/service/webhook" + "git.eeqj.de/sneak/upaas/templates" ) // Params contains dependencies for Handlers. @@ -80,6 +82,28 @@ func (h *Handlers) addGlobals( return data } +// renderTemplate executes the named template into a buffer first, then writes +// to the ResponseWriter only on success. This prevents partial/corrupt HTML +// responses when template execution fails partway through. +func (h *Handlers) renderTemplate( + writer http.ResponseWriter, + tmpl *templates.TemplateExecutor, + name string, + data any, +) { + var buf bytes.Buffer + + err := tmpl.ExecuteTemplate(&buf, name, data) + if err != nil { + h.log.Error("template execution failed", "error", err) + http.Error(writer, "Internal Server Error", http.StatusInternalServerError) + + return + } + + _, _ = buf.WriteTo(writer) +} + func (h *Handlers) respondJSON( writer http.ResponseWriter, _ *http.Request, diff --git a/internal/handlers/render_template_test.go b/internal/handlers/render_template_test.go new file mode 100644 index 0000000..98b8fc0 --- /dev/null +++ b/internal/handlers/render_template_test.go @@ -0,0 +1,73 @@ +package handlers_test + +import ( + "net/http" + "net/http/httptest" + "testing" + + "github.com/stretchr/testify/assert" +) + +// TestRenderTemplateBuffersOutput verifies that successful template rendering +// produces a complete HTML response (not partial/corrupt). +func TestRenderTemplateBuffersOutput(t *testing.T) { + t.Parallel() + + testCtx := setupTestHandlers(t) + + // The setup page is simple and has no DB dependencies + request := httptest.NewRequest(http.MethodGet, "/setup", nil) + recorder := httptest.NewRecorder() + + handler := testCtx.handlers.HandleSetupGET() + handler.ServeHTTP(recorder, request) + + assert.Equal(t, http.StatusOK, recorder.Code) + + body := recorder.Body.String() + // A properly buffered response should contain the closing tag, + // proving the full template was rendered before being sent. + assert.Contains(t, body, "") + // Should NOT contain the error text that would be appended on failure + assert.NotContains(t, body, "Internal Server Error") +} + +// TestDashboardRenderTemplateBuffersOutput verifies the dashboard handler +// also uses buffered template rendering. +func TestDashboardRenderTemplateBuffersOutput(t *testing.T) { + t.Parallel() + + testCtx := setupTestHandlers(t) + + request := httptest.NewRequest(http.MethodGet, "/", nil) + recorder := httptest.NewRecorder() + + handler := testCtx.handlers.HandleDashboard() + handler.ServeHTTP(recorder, request) + + assert.Equal(t, http.StatusOK, recorder.Code) + + body := recorder.Body.String() + assert.Contains(t, body, "") + assert.NotContains(t, body, "Internal Server Error") +} + +// TestLoginRenderTemplateBuffersOutput verifies the login handler +// uses buffered template rendering. +func TestLoginRenderTemplateBuffersOutput(t *testing.T) { + t.Parallel() + + testCtx := setupTestHandlers(t) + + request := httptest.NewRequest(http.MethodGet, "/login", nil) + recorder := httptest.NewRecorder() + + handler := testCtx.handlers.HandleLoginGET() + handler.ServeHTTP(recorder, request) + + assert.Equal(t, http.StatusOK, recorder.Code) + + body := recorder.Body.String() + assert.Contains(t, body, "") + assert.NotContains(t, body, "Internal Server Error") +} diff --git a/internal/handlers/setup.go b/internal/handlers/setup.go index fd7805f..f7e9519 100644 --- a/internal/handlers/setup.go +++ b/internal/handlers/setup.go @@ -18,11 +18,7 @@ func (h *Handlers) HandleSetupGET() http.HandlerFunc { return func(writer http.ResponseWriter, request *http.Request) { data := h.addGlobals(map[string]any{}, request) - err := tmpl.ExecuteTemplate(writer, "setup.html", data) - if err != nil { - h.log.Error("template execution failed", "error", err) - http.Error(writer, "Internal Server Error", http.StatusInternalServerError) - } + h.renderTemplate(writer, tmpl, "setup.html", data) } } @@ -62,7 +58,7 @@ func (h *Handlers) renderSetupError( "Username": username, "Error": errorMsg, }, request) - _ = tmpl.ExecuteTemplate(writer, "setup.html", data) + h.renderTemplate(writer, tmpl, "setup.html", data) } // HandleSetupPOST handles the setup form submission. diff --git a/internal/middleware/middleware.go b/internal/middleware/middleware.go index bc650b8..69fa447 100644 --- a/internal/middleware/middleware.go +++ b/internal/middleware/middleware.go @@ -111,10 +111,53 @@ func ipFromHostPort(hostPort string) string { return host } -// realIP extracts the client's real IP address from the request, -// checking proxy headers first (trusted reverse proxy like Traefik), -// then falling back to RemoteAddr. +// trustedProxyNets are RFC1918 and loopback CIDRs whose proxy headers we trust. +// +//nolint:gochecknoglobals // package-level constant nets parsed once +var trustedProxyNets = func() []*net.IPNet { + cidrs := []string{ + "10.0.0.0/8", + "172.16.0.0/12", + "192.168.0.0/16", + "127.0.0.0/8", + "::1/128", + "fc00::/7", + } + + nets := make([]*net.IPNet, 0, len(cidrs)) + + for _, cidr := range cidrs { + _, n, _ := net.ParseCIDR(cidr) + nets = append(nets, n) + } + + return nets +}() + +// isTrustedProxy reports whether ip is in an RFC1918, loopback, or ULA range. +func isTrustedProxy(ip net.IP) bool { + for _, n := range trustedProxyNets { + if n.Contains(ip) { + return true + } + } + + return false +} + +// realIP extracts the client's real IP address from the request. +// Proxy headers (X-Real-IP, X-Forwarded-For) are only trusted when the +// direct connection originates from an RFC1918/loopback address. +// Otherwise, headers are ignored and RemoteAddr is used (fail closed). func realIP(r *http.Request) string { + addr := ipFromHostPort(r.RemoteAddr) + remoteIP := net.ParseIP(addr) + + // Only trust proxy headers from private/loopback sources. + if remoteIP == nil || !isTrustedProxy(remoteIP) { + return addr + } + // 1. X-Real-IP (set by Traefik/nginx) if ip := strings.TrimSpace(r.Header.Get("X-Real-IP")); ip != "" { return ip @@ -130,7 +173,7 @@ func realIP(r *http.Request) string { } // 3. Fall back to RemoteAddr - return ipFromHostPort(r.RemoteAddr) + return addr } // CORS returns CORS middleware. diff --git a/internal/middleware/realip_test.go b/internal/middleware/realip_test.go index 8cd1632..f38aa48 100644 --- a/internal/middleware/realip_test.go +++ b/internal/middleware/realip_test.go @@ -2,6 +2,7 @@ package middleware //nolint:testpackage // tests unexported realIP function import ( "context" + "net" "net/http" "testing" ) @@ -16,54 +17,98 @@ func TestRealIP(t *testing.T) { //nolint:funlen // table-driven test xff string want string }{ + // === Trusted proxy (RFC1918 / loopback) — headers ARE honoured === { - name: "X-Real-IP takes priority", + name: "trusted: X-Real-IP from 10.x", remoteAddr: "10.0.0.1:1234", xRealIP: "203.0.113.5", xff: "198.51.100.1, 10.0.0.1", want: "203.0.113.5", }, { - name: "X-Forwarded-For used when no X-Real-IP", + name: "trusted: XFF from 10.x when no X-Real-IP", remoteAddr: "10.0.0.1:1234", xff: "198.51.100.1, 10.0.0.1", want: "198.51.100.1", }, { - name: "X-Forwarded-For single IP", + name: "trusted: XFF single IP from 10.x", remoteAddr: "10.0.0.1:1234", xff: "203.0.113.10", want: "203.0.113.10", }, { - name: "falls back to RemoteAddr", + name: "trusted: falls back to RemoteAddr (192.168.x)", remoteAddr: "192.168.1.1:5678", want: "192.168.1.1", }, { - name: "RemoteAddr without port", + name: "trusted: RemoteAddr without port", remoteAddr: "192.168.1.1", want: "192.168.1.1", }, { - name: "X-Real-IP with whitespace", + name: "trusted: X-Real-IP with whitespace from 10.x", remoteAddr: "10.0.0.1:1234", xRealIP: " 203.0.113.5 ", want: "203.0.113.5", }, { - name: "X-Forwarded-For with whitespace", + name: "trusted: XFF with whitespace from 10.x", remoteAddr: "10.0.0.1:1234", xff: " 198.51.100.1 , 10.0.0.1", want: "198.51.100.1", }, { - name: "empty X-Real-IP falls through to XFF", + name: "trusted: empty X-Real-IP falls through to XFF from 10.x", remoteAddr: "10.0.0.1:1234", xRealIP: " ", xff: "198.51.100.1", want: "198.51.100.1", }, + { + name: "trusted: loopback honours X-Real-IP", + remoteAddr: "127.0.0.1:9999", + xRealIP: "93.184.216.34", + want: "93.184.216.34", + }, + { + name: "trusted: 172.16.x honours XFF", + remoteAddr: "172.16.0.1:4321", + xff: "8.8.8.8", + want: "8.8.8.8", + }, + + // === Untrusted proxy (public IP) — headers IGNORED, use RemoteAddr === + { + name: "untrusted: X-Real-IP ignored from public IP", + remoteAddr: "203.0.113.50:1234", + xRealIP: "10.0.0.1", + want: "203.0.113.50", + }, + { + name: "untrusted: XFF ignored from public IP", + remoteAddr: "198.51.100.99:5678", + xff: "10.0.0.1, 192.168.1.1", + want: "198.51.100.99", + }, + { + name: "untrusted: both headers ignored from public IP", + remoteAddr: "8.8.8.8:443", + xRealIP: "1.2.3.4", + xff: "5.6.7.8", + want: "8.8.8.8", + }, + { + name: "untrusted: no headers, public RemoteAddr", + remoteAddr: "93.184.216.34:8080", + want: "93.184.216.34", + }, + { + name: "untrusted: public RemoteAddr without port", + remoteAddr: "93.184.216.34", + want: "93.184.216.34", + }, } for _, tt := range tests { @@ -88,3 +133,25 @@ func TestRealIP(t *testing.T) { //nolint:funlen // table-driven test }) } } + +func TestIsTrustedProxy(t *testing.T) { + t.Parallel() + + trusted := []string{"10.0.0.1", "10.255.255.255", "172.16.0.1", "172.31.255.255", + "192.168.0.1", "192.168.255.255", "127.0.0.1", "127.255.255.255", "::1"} + untrusted := []string{"8.8.8.8", "203.0.113.1", "172.32.0.1", "11.0.0.1", "2001:db8::1"} + + for _, addr := range trusted { + ip := net.ParseIP(addr) + if !isTrustedProxy(ip) { + t.Errorf("expected %s to be trusted", addr) + } + } + + for _, addr := range untrusted { + ip := net.ParseIP(addr) + if isTrustedProxy(ip) { + t.Errorf("expected %s to be untrusted", addr) + } + } +} diff --git a/static/js/app.js b/static/js/app.js index cd567a9..4829867 100644 --- a/static/js/app.js +++ b/static/js/app.js @@ -185,11 +185,12 @@ document.addEventListener("alpine:init", () => { // Track whether user wants auto-scroll (per log pane) _containerAutoScroll: true, _buildAutoScroll: true, + _pollTimer: null, init() { this.deploying = Alpine.store("utils").isDeploying(this.appStatus); this.fetchAll(); - setInterval(() => this.fetchAll(), 1000); + this._schedulePoll(); // Set up scroll listeners after DOM is ready this.$nextTick(() => { @@ -198,6 +199,15 @@ document.addEventListener("alpine:init", () => { }); }, + _schedulePoll() { + if (this._pollTimer) clearTimeout(this._pollTimer); + const interval = Alpine.store("utils").isDeploying(this.appStatus) ? 1000 : 10000; + this._pollTimer = setTimeout(() => { + this.fetchAll(); + this._schedulePoll(); + }, interval); + }, + _initScrollTracking(el, flag) { if (!el) return; el.addEventListener('scroll', () => { @@ -207,18 +217,36 @@ document.addEventListener("alpine:init", () => { fetchAll() { this.fetchAppStatus(); - this.fetchContainerLogs(); - this.fetchBuildLogs(); + // Only fetch logs when the respective pane is visible + if (this.$refs.containerLogsWrapper && this._isElementVisible(this.$refs.containerLogsWrapper)) { + this.fetchContainerLogs(); + } + if (this.showBuildLogs && this.$refs.buildLogsWrapper && this._isElementVisible(this.$refs.buildLogsWrapper)) { + this.fetchBuildLogs(); + } this.fetchRecentDeployments(); }, + _isElementVisible(el) { + if (!el) return false; + // Check if element is in viewport (roughly) + const rect = el.getBoundingClientRect(); + return rect.bottom > 0 && rect.top < window.innerHeight; + }, + async fetchAppStatus() { try { const res = await fetch(`/apps/${this.appId}/status`); const data = await res.json(); + const wasDeploying = this.deploying; this.appStatus = data.status; this.deploying = Alpine.store("utils").isDeploying(data.status); + // Re-schedule polling when deployment state changes + if (this.deploying !== wasDeploying) { + this._schedulePoll(); + } + if ( data.latestDeploymentID && data.latestDeploymentID !== this.currentDeploymentId @@ -429,7 +457,18 @@ document.addEventListener("alpine:init", () => { } this.fetchAppStatus(); - setInterval(() => this.fetchAppStatus(), 1000); + this._scheduleStatusPoll(); + }, + + _statusPollTimer: null, + + _scheduleStatusPoll() { + if (this._statusPollTimer) clearTimeout(this._statusPollTimer); + const interval = this.isDeploying ? 1000 : 10000; + this._statusPollTimer = setTimeout(() => { + this.fetchAppStatus(); + this._scheduleStatusPoll(); + }, interval); }, async fetchAppStatus() { @@ -464,6 +503,7 @@ document.addEventListener("alpine:init", () => { // Update deploying state based on latest deployment status if (deploying && !this.isDeploying) { this.isDeploying = true; + this._scheduleStatusPoll(); // Switch to fast polling } else if (!deploying && this.isDeploying) { // Deployment finished - reload to show final state this.isDeploying = false;