From ca15f52dcce8102014f44135a6bf2ae5e72d09f2 Mon Sep 17 00:00:00 2001
From: sneak <sneak@sneak.berlin>
Date: Fri, 7 Aug 2026 21:02:07 +0700
Subject: [PATCH] test: require Secure/HttpOnly/SameSite on all session cookies

Add a failing test asserting that every cookie written by the session
manager (both the CreateSession set-cookie path and the ClearSession
delete-cookie path) carries HttpOnly, Secure, and a SameSite mode of
Lax or stricter, regardless of constructor arguments. Session cookies
carry authentication state and must never be sent over plaintext HTTP.

Currently fails for the constructor secure=false case, which produces
cookies without the Secure attribute (gosec G124 at
internal/session/session.go:84 and :128).
---
 .../session/session_cookie_attributes_test.go | 92 +++++++++++++++++++
 1 file changed, 92 insertions(+)
 create mode 100644 internal/session/session_cookie_attributes_test.go

diff --git a/internal/session/session_cookie_attributes_test.go b/internal/session/session_cookie_attributes_test.go
new file mode 100644
index 0000000..8f2ddd6
--- /dev/null
+++ b/internal/session/session_cookie_attributes_test.go
@@ -0,0 +1,92 @@
+package session
+
+import (
+	"net/http"
+	"net/http/httptest"
+	"testing"
+)
+
+// TestSessionCookieAttributesAlwaysSecure verifies that every cookie
+// emitted by the session manager carries HttpOnly, Secure, and a
+// SameSite mode of Lax or stricter, regardless of how the manager was
+// constructed. Session cookies contain the authentication state and
+// must never be exposed to script (HttpOnly), sent over plaintext HTTP
+// (Secure), or attached to cross-site requests (SameSite). No
+// constructor argument may weaken these attributes.
+//
+// This covers both cookie-writing paths: CreateSession (the login
+// set-cookie path) and ClearSession (the logout delete-cookie path).
+func TestSessionCookieAttributesAlwaysSecure(t *testing.T) {
+	for _, constructorBoolArg := range []bool{false, true} {
+		mgr, err := NewManager("test-signing-key-12345", constructorBoolArg)
+		if err != nil {
+			t.Fatalf("NewManager() error = %v", err)
+		}
+
+		writePaths := []struct {
+			name      string
+			setCookie func(t *testing.T, w http.ResponseWriter)
+		}{
+			{
+				name: "CreateSession",
+				setCookie: func(t *testing.T, w http.ResponseWriter) {
+					t.Helper()
+					if err := mgr.CreateSession(w); err != nil {
+						t.Fatalf("CreateSession() error = %v", err)
+					}
+				},
+			},
+			{
+				name: "ClearSession",
+				setCookie: func(t *testing.T, w http.ResponseWriter) {
+					t.Helper()
+					mgr.ClearSession(w)
+				},
+			},
+		}
+
+		for _, writePath := range writePaths {
+			testName := writePath.name
+			if constructorBoolArg {
+				testName += "/constructorBoolArg=true"
+			} else {
+				testName += "/constructorBoolArg=false"
+			}
+
+			t.Run(testName, func(t *testing.T) {
+				w := httptest.NewRecorder()
+				writePath.setCookie(t, w)
+
+				var sessionCookie *http.Cookie
+				for _, c := range w.Result().Cookies() {
+					if c.Name == CookieName {
+						sessionCookie = c
+
+						break
+					}
+				}
+
+				if sessionCookie == nil {
+					t.Fatalf("no cookie named %q was set", CookieName)
+				}
+
+				t.Logf("cookie attributes: HttpOnly=%v Secure=%v SameSite=%v",
+					sessionCookie.HttpOnly, sessionCookie.Secure, sessionCookie.SameSite)
+
+				if !sessionCookie.HttpOnly {
+					t.Error("session cookie must have HttpOnly set")
+				}
+
+				if !sessionCookie.Secure {
+					t.Error("session cookie must have Secure set")
+				}
+
+				if sessionCookie.SameSite != http.SameSiteLaxMode &&
+					sessionCookie.SameSite != http.SameSiteStrictMode {
+					t.Errorf("session cookie SameSite = %v, want Lax (%v) or Strict (%v)",
+						sessionCookie.SameSite, http.SameSiteLaxMode, http.SameSiteStrictMode)
+				}
+			})
+		}
+	}
+}