sneak/pixa
1
0
Fork 0
Code Issues 48 Pull Requests 4 Actions Packages Projects Releases Wiki Activity

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
Conversation 4 Commits 3 Files Changed 5
+120 -38
3 Commits
Author SHA1 Message Date
sneak cb9e14eee7 refactor: drop the ignored secure toggle parameter from session.NewManager
check / check (push) Successful in 1m48s
Details 
Per review on PR #48: pre-1.0 there is no installed base to keep
compiling against, so remove the dead parameter in one pass instead of
deferring. NewManager now takes only the signing key. Update the
handlers.go call site and its stale comment, mechanically update the
NewManager call shapes in the session tests (assertions unchanged),
collapse the now-meaningless constructor-argument loop in the cookie
attributes test, and drop the moot P1 Future Step from TODO.md.

Reviewer-directed test call-site updates; cookie behavior is unchanged
from the previous commit (always Secure/HttpOnly/SameSite=Strict).
 2026-08-07 22:24:15 +07:00
sneak 02ca16a68a fix: always set Secure/HttpOnly/SameSite on session cookies (closes #47)
check / check (push) Successful in 2m4s
Details 
Resolve the two remaining gosec G124 findings (internal/session/
session.go:84 and :128): session cookies are now unconditionally
Secure, HttpOnly, and SameSite=Strict on both the CreateSession
set-cookie path and the ClearSession delete-cookie path. gosec requires
these attributes to be constant, and there is no legitimate
configuration in which the authentication cookie should be weaker, so
the former secure toggle (wired to !config.Debug) is removed rather
than kept as a variable.

The toggle parameter on NewManager is retained as an ignored blank
parameter so existing call sites (including tests) keep compiling;
removing it is tracked as a Future Step in TODO.md. Local development
over http://localhost keeps working because browsers treat localhost as
a trustworthy origin and accept Secure cookies there.

Update TODO.md per its Workflow section: record this step as completed,
promote the manual auth/URL-flow test pass to Next Step, and correct
the stale Status text (make check is now green).
 2026-08-07 21:14:16 +07:00
sneak ca15f52dcc 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).
 2026-08-07 21:02:07 +07:00