TODO.md

@@ -10,22 +10,27 @@
 
 # Status
 
-pre-1.0. No git tags exist. main (6b4a1d7, 2026-04-07) is current with
+pre-1.0. No git tags exist. Recent work extracted the internal/magic,
-origin; recent work extracted the internal/magic and internal/allowlist
+internal/allowlist, internal/httpfetcher, and internal/signature
-packages. The 10 gosec findings from the 2026-07-06 morning survey are
+packages. The gosec findings from the 2026-07-06 survey are resolved:
-NOT fixed on main: the newest gosec/lint fix commits are from
+the last two open findings (G124, session cookie attributes in
-2026-02-25 (85729d9, ce6db76) and nothing since touches gosec, so those
+internal/session) are fixed as of this change, so `make check` is green
-findings remain open.
+on main.
 
 # Next Step
 
-Fix the 10 open gosec lint findings so make check passes on main again
+P0: manual test pass of the auth and encrypted URL flows, then commit
-(main must always be green; no gosec fix commits have landed since
+the checked-off results to TODO.md: visit / and see the login form;
-2026-02-25). No blanket suppressions; fix or justify each finding
+wrong key shows an error; correct signing key shows the generator form;
-individually.
+a generated encrypted URL serves the image; an expired URL (short TTL)
+returns 410; logout redirects back to login
 
 # Completed Steps
 
+- 2026-08-07 fix the two remaining gosec findings (G124 in
+  internal/session): session cookies now always carry
+  Secure/HttpOnly/SameSite=Strict on both the set and clear paths;
+  `make check` green (closes #47)
 - 2026-07-07 Adopted scripts-to-rule-them-all: `script/` entrypoints,
   Makefile shims, README Entrypoints section
 - 2026-04-07 extract magic byte detection into internal/magic (#42)
@@ -48,11 +53,6 @@ individually.
 
 # Future Steps
 
-- P0: manual test pass of the auth and encrypted URL flows, then
-  commit the checked-off results to TODO.md: visit / and see the login
-  form; wrong key shows an error; correct signing key shows the
-  generator form; a generated encrypted URL serves the image; an
-  expired URL (short TTL) returns 410; logout redirects back to login
 - P0: implement cache size management and eviction so the disk cannot
   fill up
 - P0: validate configuration on startup, fail fast on bad config

internal/handlers/handlers.go

@@ -94,8 +94,9 @@ func (s *Handlers) initImageService() error {
 	s.imgSvc = svc
 	s.log.Info("image service initialized")
 
-	// Initialize session manager (signing key is validated at config load time)
+	// Initialize session manager (signing key is validated at config load
-	sessMgr, err := session.NewManager(s.config.SigningKey, !s.config.Debug)
+	// time). Session cookies are always Secure/HttpOnly/SameSite=Strict.
+	sessMgr, err := session.NewManager(s.config.SigningKey)
 	if err != nil {
 		return err
 	}

internal/session/session.go

@@ -36,14 +36,16 @@ type Data struct {
 
 // Manager handles session creation and validation using encrypted cookies.
 type Manager struct {
-	sc       *securecookie.SecureCookie
+	sc *securecookie.SecureCookie
-	secure   bool // Set Secure flag on cookies (should be true in production)
-	sameSite http.SameSite
 }
 
 // NewManager creates a session manager with keys derived from the signing key.
-// Set secure=true in production to require HTTPS for cookies.
+//
-func NewManager(signingKey string, secure bool) (*Manager, error) {
+// Session cookies always carry the Secure, HttpOnly, and SameSite=Strict
+// attributes; this cannot be configured. Browsers treat http://localhost as a
+// trustworthy origin and accept Secure cookies there, so local development
+// keeps working.
+func NewManager(signingKey string) (*Manager, error) {
 	masterKey := []byte(signingKey)
 
 	// Derive separate keys for HMAC (hash) and encryption (block)
@@ -61,9 +63,7 @@ func NewManager(signingKey string, secure bool) (*Manager, error) {
 	sc.MaxAge(int(SessionTTL.Seconds()))
 
 	return &Manager{
-		sc:       sc,
+		sc: sc,
-		secure:   secure,
-		sameSite: http.SameSiteStrictMode,
 	}, nil
 }
 
@@ -87,8 +87,8 @@ func (m *Manager) CreateSession(w http.ResponseWriter) error {
 		Path:     "/",
 		MaxAge:   int(SessionTTL.Seconds()),
 		HttpOnly: true,
-		Secure:   m.secure,
+		Secure:   true,
-		SameSite: m.sameSite,
+		SameSite: http.SameSiteStrictMode,
 	})
 
 	return nil
@@ -131,8 +131,8 @@ func (m *Manager) ClearSession(w http.ResponseWriter) {
 		Path:     "/",
 		MaxAge:   -1, // Delete immediately
 		HttpOnly: true,
-		Secure:   m.secure,
+		Secure:   true,
-		SameSite: m.sameSite,
+		SameSite: http.SameSiteStrictMode,
 	})
 }
 

internal/session/session_cookie_attributes_test.go

@@ -0,0 +1,82 @@
+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. 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). Nothing 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) {
+	mgr, err := NewManager("test-signing-key-12345")
+	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 {
+		t.Run(writePath.name, 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)
+			}
+		})
+	}
+}

internal/session/session_test.go

@@ -8,7 +8,7 @@ import (
 )
 
 func TestManager_CreateAndValidate(t *testing.T) {
-	mgr, err := NewManager("test-signing-key-12345", false)
+	mgr, err := NewManager("test-signing-key-12345")
 	if err != nil {
 		t.Fatalf("NewManager() error = %v", err)
 	}
@@ -57,7 +57,7 @@ func TestManager_CreateAndValidate(t *testing.T) {
 }
 
 func TestManager_ValidateSession_NoCookie(t *testing.T) {
-	mgr, _ := NewManager("test-signing-key-12345", false)
+	mgr, _ := NewManager("test-signing-key-12345")
 
 	req := httptest.NewRequest(http.MethodGet, "/", nil)
 
@@ -72,7 +72,7 @@ func TestManager_ValidateSession_NoCookie(t *testing.T) {
 }
 
 func TestManager_ValidateSession_TamperedCookie(t *testing.T) {
-	mgr, _ := NewManager("test-signing-key-12345", false)
+	mgr, _ := NewManager("test-signing-key-12345")
 
 	req := httptest.NewRequest(http.MethodGet, "/", nil)
 	req.AddCookie(&http.Cookie{
@@ -91,8 +91,8 @@ func TestManager_ValidateSession_TamperedCookie(t *testing.T) {
 }
 
 func TestManager_ValidateSession_WrongKey(t *testing.T) {
-	mgr1, _ := NewManager("signing-key-1", false)
+	mgr1, _ := NewManager("signing-key-1")
-	mgr2, _ := NewManager("signing-key-2", false)
+	mgr2, _ := NewManager("signing-key-2")
 
 	// Create session with mgr1
 	w := httptest.NewRecorder()
@@ -118,7 +118,7 @@ func TestManager_ValidateSession_WrongKey(t *testing.T) {
 }
 
 func TestManager_ClearSession(t *testing.T) {
-	mgr, _ := NewManager("test-signing-key-12345", false)
+	mgr, _ := NewManager("test-signing-key-12345")
 
 	w := httptest.NewRecorder()
 	mgr.ClearSession(w)
@@ -144,7 +144,7 @@ func TestManager_ClearSession(t *testing.T) {
 }
 
 func TestManager_IsAuthenticated(t *testing.T) {
-	mgr, _ := NewManager("test-signing-key-12345", false)
+	mgr, _ := NewManager("test-signing-key-12345")
 
 	// No session - should return false
 	req := httptest.NewRequest(http.MethodGet, "/", nil)
@@ -175,8 +175,7 @@ func TestManager_IsAuthenticated(t *testing.T) {
 }
 
 func TestManager_CookieAttributes(t *testing.T) {
-	// Test with secure=true
+	mgr, _ := NewManager("test-key")
-	mgr, _ := NewManager("test-key", true)
 
 	w := httptest.NewRecorder()
 	_ = mgr.CreateSession(w)