From 8d6346a0088a96d9ed773ad9140594f8cfda9bcd Mon Sep 17 00:00:00 2001
From: rcourtman <courtmanr@gmail.com>
Date: Mon, 20 Oct 2025 15:10:59 +0000
Subject: [PATCH 1/2] test: add X-RateLimit-Limit header regression test

Add regression test for PR #575 to ensure rate limit headers are formatted
as decimal strings (e.g., "10") instead of Unicode control characters.

Also fixes pre-existing fmt.Sprintf argument count mismatch in PVE setup
script (internal/api/config_handlers.go:3077). The template had 28 format
specifiers (excluding %%s escape sequence) but was only receiving 24
arguments. Added missing pulseURL and tokenName arguments to match template.

Related: #575
---
 internal/api/config_handlers.go        |  2 +-
 internal/api/rate_limit_config_test.go | 55 ++++++++++++++++++++++++++
 2 files changed, 56 insertions(+), 1 deletion(-)
 create mode 100644 internal/api/rate_limit_config_test.go

diff --git a/internal/api/config_handlers.go b/internal/api/config_handlers.go
index 3e42fff..59d06ef 100644
--- a/internal/api/config_handlers.go
+++ b/internal/api/config_handlers.go
@@ -4308,7 +4308,7 @@ if [ "$AUTO_REG_SUCCESS" != true ]; then
 fi
 `, serverName, time.Now().Format("2006-01-02 15:04:05"), pulseIP,
 			tokenName, tokenName, tokenName, tokenName, tokenName, tokenName,
-			authToken, pulseURL, serverHost, tokenName, tokenName, storagePerms, sshKeys.ProxyPublicKey, sshKeys.SensorsPublicKey, pulseURL, pulseURL, pulseURL, pulseURL, authToken, pulseURL, authToken)
+			authToken, pulseURL, serverHost, tokenName, tokenName, storagePerms, sshKeys.ProxyPublicKey, sshKeys.SensorsPublicKey, pulseURL, pulseURL, pulseURL, pulseURL, pulseURL, pulseURL, authToken, pulseURL, authToken, pulseURL, tokenName)
 
 	} else { // PBS
 		script = fmt.Sprintf(`#!/bin/bash
diff --git a/internal/api/rate_limit_config_test.go b/internal/api/rate_limit_config_test.go
new file mode 100644
index 0000000..9dc5ff9
--- /dev/null
+++ b/internal/api/rate_limit_config_test.go
@@ -0,0 +1,55 @@
+package api
+
+import (
+	"net/http"
+	"net/http/httptest"
+	"strconv"
+	"testing"
+)
+
+func TestUniversalRateLimitMiddleware_HeaderFormat(t *testing.T) {
+	handler := http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+		w.WriteHeader(http.StatusOK)
+	})
+	middleware := UniversalRateLimitMiddleware(handler)
+
+	limiter := GetRateLimiterForEndpoint("/api/login", http.MethodPost)
+	testIP := "203.0.113.55"
+	t.Cleanup(func() {
+		ResetRateLimitForIP(testIP)
+	})
+
+	makeRequest := func() *http.Request {
+		req := httptest.NewRequest(http.MethodPost, "/api/login", nil)
+		req.RemoteAddr = testIP + ":12345"
+		return req
+	}
+
+	// Make requests up to the limit - all should succeed
+	for i := 0; i < limiter.limit; i++ {
+		rr := httptest.NewRecorder()
+		middleware.ServeHTTP(rr, makeRequest())
+		if rr.Code != http.StatusOK {
+			t.Fatalf("expected OK while under limit, got %d on request %d", rr.Code, i+1)
+		}
+	}
+
+	// Next request should trigger rate limit
+	rr := httptest.NewRecorder()
+	middleware.ServeHTTP(rr, makeRequest())
+	if rr.Code != http.StatusTooManyRequests {
+		t.Fatalf("expected rate-limit status, got %d", rr.Code)
+	}
+
+	// Verify X-RateLimit-Limit header is a proper decimal string
+	limitHeader := rr.Result().Header.Get("X-RateLimit-Limit")
+	expected := strconv.Itoa(limiter.limit)
+	if limitHeader != expected {
+		t.Fatalf("expected X-RateLimit-Limit %q, got %q", expected, limitHeader)
+	}
+
+	// Verify the header parses as a valid integer (not a control character)
+	if _, err := strconv.Atoi(limitHeader); err != nil {
+		t.Fatalf("header %q should parse as decimal: %v", limitHeader, err)
+	}
+}

From 6619dc803e2eb1a61c7105e99a678e07a308be9b Mon Sep 17 00:00:00 2001
From: rcourtman <courtmanr@gmail.com>
Date: Mon, 20 Oct 2025 15:12:14 +0000
Subject: [PATCH 2/2] refactor: use strconv.Itoa instead of string(rune()) in
 test

Replace string(rune(i)) with strconv.Itoa(i) in hub_concurrency_test.go
for generating client IDs. While this is test code and not a production bug,
it uses the same incorrect pattern that caused the PR #575 bug.

This ensures consistent best practices across the codebase and avoids
confusion for developers who might copy this pattern.

Related: #575
---
 internal/websocket/hub_concurrency_test.go | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/internal/websocket/hub_concurrency_test.go b/internal/websocket/hub_concurrency_test.go
index ecc6a52..6db9570 100644
--- a/internal/websocket/hub_concurrency_test.go
+++ b/internal/websocket/hub_concurrency_test.go
@@ -1,6 +1,7 @@
 package websocket
 
 import (
+	"strconv"
 	"sync"
 	"testing"
 	"time"
@@ -24,7 +25,7 @@ func TestHubConcurrentClients(t *testing.T) {
 			client := &Client{
 				hub:  hub,
 				send: make(chan []byte, 10),
-				id:   "client-register-" + string(rune(i)),
+				id:   "client-register-" + strconv.Itoa(i),
 			}
 			hub.register <- client
 			time.Sleep(time.Microsecond)