fix: set Secure/HttpOnly/SameSite on session cookies (closes #47) #48

Merged
sneak merged 3 commits from fix/gosec-findings into main 2026-08-07 17:41:03 +02:00
3 changed files with 36 additions and 29 deletions
Showing only changes of commit 02ca16a68a - Show all commits

33
TODO.md
View File

@@ -10,22 +10,27 @@
# Status
pre-1.0. No git tags exist. main (6b4a1d7, 2026-04-07) is current with
origin; recent work extracted the internal/magic and internal/allowlist
packages. The 10 gosec findings from the 2026-07-06 morning survey are
NOT fixed on main: the newest gosec/lint fix commits are from
2026-02-25 (85729d9, ce6db76) and nothing since touches gosec, so those
findings remain open.
pre-1.0. No git tags exist. Recent work extracted the internal/magic,
internal/allowlist, internal/httpfetcher, and internal/signature
packages. The gosec findings from the 2026-07-06 survey are resolved:
the last two open findings (G124, session cookie attributes in
internal/session) are fixed as of this change, so `make check` is green
on main.
# Next Step
Fix the 10 open gosec lint findings so make check passes on main again
(main must always be green; no gosec fix commits have landed since
2026-02-25). No blanket suppressions; fix or justify each finding
individually.
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
# 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,14 +53,12 @@ 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
- P1: remove the ignored former secure toggle parameter from
session.NewManager and update its call sites (requires touching
existing tests; needs approval per repo rules)
- P1: implement blocked networks configuration to extend SSRF
protection
- P1: rate limit global concurrent upstream fetches to prevent

View File

@@ -94,8 +94,10 @@ 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)
sessMgr, err := session.NewManager(s.config.SigningKey, !s.config.Debug)
// Initialize session manager (signing key is validated at config load
// time). The second argument is ignored: session cookies are always
// Secure/HttpOnly/SameSite=Strict.
sessMgr, err := session.NewManager(s.config.SigningKey, true)
if err != nil {
return err
}

View File

@@ -37,13 +37,17 @@ type Data struct {
// Manager handles session creation and validation using encrypted cookies.
type Manager struct {
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. The second parameter is the former secure toggle: it is
// ignored and retained only so existing call sites keep compiling; it will be
// removed in a follow-up change.
func NewManager(signingKey string, _ bool) (*Manager, error) {
masterKey := []byte(signingKey)
// Derive separate keys for HMAC (hash) and encryption (block)
@@ -62,8 +66,6 @@ func NewManager(signingKey string, secure bool) (*Manager, error) {
return &Manager{
sc: sc,
secure: secure,
sameSite: http.SameSiteStrictMode,
}, nil
}
@@ -87,8 +89,8 @@ func (m *Manager) CreateSession(w http.ResponseWriter) error {
Path: "/",
MaxAge: int(SessionTTL.Seconds()),
HttpOnly: true,
Secure: m.secure,
SameSite: m.sameSite,
Secure: true,
SameSite: http.SameSiteStrictMode,
})
return nil
@@ -131,8 +133,8 @@ func (m *Manager) ClearSession(w http.ResponseWriter) {
Path: "/",
MaxAge: -1, // Delete immediately
HttpOnly: true,
Secure: m.secure,
SameSite: m.sameSite,
Secure: true,
SameSite: http.SameSiteStrictMode,
})
}