Skip to content

Allow provide session max life #3567

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
fiftin wants to merge 1 commit into
base: develop
Choose a base branch
from
Open

Allow provide session max life #3567

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions api/auth.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -47,8 +47,8 @@ func getSession(r *http.Request) (*db.Session, bool) {
return nil, false
}

if time.Since(session.LastActive).Hours() > 7*24 {
// more than week old unused session
if time.Since(session.LastActive).Hours() > float64(util.Config.Auth.MaxSessionLifeHours) {
Copy link
Copy Markdown

@chatgpt-codex-connector chatgpt-codex-connector Bot Jan 19, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Badge Avoid defaulting MaxSessionLifeHours to zero

If auth.max_session_life_hours is not set in config/env, MaxSessionLifeHours remains 0 because loadDefaultsToObject doesn’t descend into pointer-to-struct fields (like AuthConfig), so the new default tag never applies. With a zero value, time.Since(...).Hours() > 0 becomes true for essentially every existing session, and this code expires sessions immediately on the next request. This is a regression for installs that don’t set the new field (including the default setup flow which doesn’t write an auth section). Consider explicitly handling a zero value here or ensuring defaults are applied to pointer fields.

Useful? React with 👍 / 👎.

Copy link
Copy Markdown
Collaborator Author

@fiftin fiftin Feb 16, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@codex fix it

Copy link
Copy Markdown

@chatgpt-codex-connector chatgpt-codex-connector Bot Feb 16, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Summary

  • Added a safe config accessor GetMaxSessionLifeHours() with a default fallback (168) when Config.Auth is nil or when max_session_life_hours is unset/invalid (<= 0), preventing nil dereference and accidental immediate session expiry. Also clarified the field comment wording to match actual behavior. util/config_auth.goL16-L34
  • Updated API session validation to use util.GetMaxSessionLifeHours() instead of directly dereferencing util.Config.Auth.MaxSessionLifeHours. api/auth.goL50-L56
  • Updated service-layer session validation similarly to use util.GetMaxSessionLifeHours(). services/session_svc.goL52-L56
  • Committed changes on branch work in commit 0ca8b1a, and created a follow-up PR via make_pr titled "[Follow-up] Fix session lifetime fallback when auth config is unset".

Testing

  • ✅ gofmt -w api/auth.go services/session_svc.go util/config_auth.go
  • ✅ go test ./services ./util
  • ⚠️ go test ./api ./services ./util (fails in this environment because api/router.go expects public/* assets not present during test setup)
  • ⚠️ python - <<'PY' ... urllib.request.urlopen('https://duckduckgo.com/html/?q=golang struct tag validate gt=0') ... PY (web search attempt blocked by proxy tunnel 403 Forbidden)

View task →

Copy link

Copilot AI Jan 19, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Potential nil pointer dereference. If util.Config.Auth is nil, this code will panic when trying to access MaxSessionLifeHours. The Auth field in ConfigType is defined as a pointer (*AuthConfig) and is not initialized in NewConfigType(). Consider adding a nil check or ensuring Auth is initialized with default values during config initialization.

Suggested change
if time.Since(session.LastActive).Hours() > float64(util.Config.Auth.MaxSessionLifeHours) {
if util.Config.Auth != nil && util.Config.Auth.MaxSessionLifeHours > 0 &&
time.Since(session.LastActive).Hours() > float64(util.Config.Auth.MaxSessionLifeHours) {

Copilot uses AI. Check for mistakes.
// session expired due to inactivity
// destroy.
if err = helpers.Store(r).ExpireSession(userID, sessionID); err != nil {
// it is internal error, it doesn't concern the user
Expand Down
4 changes: 2 additions & 2 deletions pro/go.mod
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -40,10 +40,10 @@ require (
github.com/sirupsen/logrus v1.9.3 // indirect
github.com/skeema/knownhosts v1.3.1 // indirect
github.com/xanzy/ssh-agent v0.3.3 // indirect
golang.org/x/crypto v0.45.0 // indirect
golang.org/x/crypto v0.46.0 // indirect
golang.org/x/exp v0.0.0-20250620022241-b7579e27df2b // indirect
golang.org/x/net v0.47.0 // indirect
golang.org/x/sys v0.38.0 // indirect
golang.org/x/sys v0.39.0 // indirect
Copy link

Copilot AI Jan 19, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This dependency update appears unrelated to the session lifetime configuration feature. Consider separating dependency updates into a different pull request for better change isolation and easier review.

Suggested change
golang.org/x/sys v0.39.0 // indirect

Copilot uses AI. Check for mistakes.
gopkg.in/natefinch/lumberjack.v2 v2.2.1 // indirect
gopkg.in/warnings.v0 v0.1.2 // indirect
modernc.org/libc v1.66.10 // indirect
Expand Down
2 changes: 2 additions & 0 deletions pro/go.sum
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -120,6 +120,7 @@ go.etcd.io/bbolt v1.4.1/go.mod h1:c8zu2BnXWTu2XM4XcICtbGSl9cFwsXtcf9zLt2OncM8=
golang.org/x/crypto v0.0.0-20220622213112-05595931fe9d/go.mod h1:IxCIyHEi3zRg3s0A5j5BB6A9Jmi73HwBIUl50j+osU4=
golang.org/x/crypto v0.45.0 h1:jMBrvKuj23MTlT0bQEOBcAE0mjg8mK9RXFhRH6nyF3Q=
golang.org/x/crypto v0.45.0/go.mod h1:XTGrrkGJve7CYK7J8PEww4aY7gM3qMCElcJQ8n8JdX4=
golang.org/x/crypto v0.46.0/go.mod h1:Evb/oLKmMraqjZ2iQTwDwvCtJkczlDuTmdJXoZVzqU0=
golang.org/x/exp v0.0.0-20250620022241-b7579e27df2b h1:M2rDM6z3Fhozi9O7NWsxAkg/yqS/lQJ6PmkyIV3YP+o=
golang.org/x/exp v0.0.0-20250620022241-b7579e27df2b/go.mod h1:3//PLf8L/X+8b4vuAfHzxeRUl04Adcb341+IGKfnqS8=
golang.org/x/mod v0.27.0 h1:kb+q2PyFnEADO2IEF935ehFUXlWiNjJWtRNgBLSfbxQ=
Expand All @@ -138,6 +139,7 @@ golang.org/x/sys v0.0.0-20220715151400-c0bba94af5f8/go.mod h1:oPkhp1MJrh7nUepCBc
golang.org/x/sys v0.6.0/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg=
golang.org/x/sys v0.38.0 h1:3yZWxaJjBmCWXqhN1qh02AkOnCQ1poK6oF+a7xWL6Gc=
golang.org/x/sys v0.38.0/go.mod h1:OgkHotnGiDImocRcuBABYBEXf8A9a87e/uXjp9XT3ks=
golang.org/x/sys v0.39.0/go.mod h1:OgkHotnGiDImocRcuBABYBEXf8A9a87e/uXjp9XT3ks=
golang.org/x/term v0.0.0-20201126162022-7de9c90e9dd1/go.mod h1:bj7SfCRtBDWHUb9snDiAeCFNEtKQo2Wmx5Cou7ajbmo=
golang.org/x/term v0.37.0 h1:8EGAD0qCmHYZg6J17DvsMy9/wJ7/D/4pV/wfnld5lTU=
golang.org/x/term v0.37.0/go.mod h1:5pB4lxRNYYVZuTLmy8oR2BH8dflOR+IbTYFD8fi3254=
Expand Down
4 changes: 2 additions & 2 deletions services/session_svc.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -49,8 +49,8 @@ func (s *sessionServiceImpl) GetSession(cookie http.Cookie) (*db.Session, bool)
return nil, false
}

if time.Since(session.LastActive).Hours() > 7*24 {
// more than week old unused session
if time.Since(session.LastActive).Hours() > float64(util.Config.Auth.MaxSessionLifeHours) {
Copy link

Copilot AI Jan 19, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Potential nil pointer dereference. If util.Config.Auth is nil, this code will panic when trying to access MaxSessionLifeHours. The Auth field in ConfigType is defined as a pointer (*AuthConfig) and is not initialized in NewConfigType(). Consider adding a nil check or ensuring Auth is initialized with default values during config initialization.

Suggested change
if time.Since(session.LastActive).Hours() > float64(util.Config.Auth.MaxSessionLifeHours) {
if util.Config.Auth != nil && time.Since(session.LastActive).Hours() > float64(util.Config.Auth.MaxSessionLifeHours) {

Copilot uses AI. Check for mistakes.
// session expired due to inactivity
// destroy.
if err = s.sessionRepo.ExpireSession(userID, sessionID); err != nil {
// it is internal error, it doesn't concern the user
Expand Down
5 changes: 5 additions & 0 deletions util/config_auth.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,4 +16,9 @@ type EmailAuthConfig struct {
type AuthConfig struct {
Totp *TotpConfig `json:"totp,omitempty"`
Email *EmailAuthConfig `json:"email,omitempty"`

// MaxSessionLifeHours defines the maximum lifetime of a session in hours.
// After this period of inactivity, the session will expire.
Copy link

Copilot AI Jan 19, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The documentation is misleading. It states "After this period of inactivity, the session will expire" but the implementation actually uses the total time since the last active timestamp, not a rolling window of inactivity. The comment should clarify that this is the maximum time since the session was last active, not a period of continuous inactivity.

Suggested change
// After this period of inactivity, the session will expire.
// When the time elapsed since the session was last active exceeds this value, the session will expire.

Copilot uses AI. Check for mistakes.
// Default is 168 hours (7 days).
MaxSessionLifeHours int `json:"max_session_life_hours,omitempty" default:"168" env:"SEMAPHORE_MAX_SESSION_LIFE_HOURS"`
Copy link

Copilot AI Jan 19, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Missing input validation for MaxSessionLifeHours. Zero or negative values would cause sessions to never expire or behave unexpectedly. Consider adding a validation rule in the struct tag (similar to other fields like Port and MaxParallelTasks in config.go) to ensure the value is positive, or add validation logic in the validateConfig function.

Suggested change
MaxSessionLifeHours int `json:"max_session_life_hours,omitempty" default:"168" env:"SEMAPHORE_MAX_SESSION_LIFE_HOURS"`
MaxSessionLifeHours int `json:"max_session_life_hours,omitempty" default:"168" env:"SEMAPHORE_MAX_SESSION_LIFE_HOURS" validate:"gt=0"`

Copilot uses AI. Check for mistakes.
}
Loading