From 8c1f5e7d4c4c9f053bb1b30bc6216d004d8032f1 Mon Sep 17 00:00:00 2001 From: Mriganka Date: Fri, 20 Feb 2026 23:44:30 +0530 Subject: [PATCH 1/3] Security and Bug Fixes MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Fixes Applied: Issue 8 — parseState() panic (DoS) - backend/pkg/server/services/auth.go:614-622 — moved the len(stateJSON) <= signatureLen bounds check before the slice stateJSON[:signatureLen], eliminating the panic on malformed short input. Issue 11 — Authorization string typos (broken ACL) - backend/pkg/server/services/users.go — removed trailing ' from all 7 privilege strings ("users.view'" → "users.view", and same for create, edit, delete). These typos caused every privilege check to silently fail. - backend/pkg/server/services/roles.go — same fix for "roles.view'". Issue 5 — OAuth GET callback missing state validation (CSRF) - backend/pkg/server/services/auth.go:327-332 — AuthLoginGetCallback now validates the state query param against the session cookie value, matching the POST handler's behavior. Issue 6 — Session expiry not enforced in middleware - backend/pkg/server/auth/auth_middleware.go:102-109 — tryUserCookieAuthentication now checks time.Now().Unix() > exp and returns authResultFail for expired sessions, not just the /info route. Issue 7 — Missing SameSite cookie attribute - backend/pkg/server/services/auth.go — added SameSite: http.SameSiteLaxMode to all four sessions.Options{} blocks and the http.Cookie in setCallbackCookie. Issue 1 (partial) — password_change_required not enforced server-side - backend/pkg/server/response/errors.go — added ErrAuthPasswordChangeRequired (HTTP 403). - backend/pkg/server/services/auth.go:125-129 — AuthLogin now returns 403 when user.PasswordChangeRequired == true, blocking login until the password is changed. Issue 9 — Browser tool HTTP client has no timeout - backend/pkg/tools/browser.go:425-428 — added Timeout: 30 * time.Second to the http.Client in callScraper, preventing indefinite hangs. --- .claude/settings.local.json | 7 +++++++ backend/pkg/server/auth/auth_middleware.go | 9 +++++++++ backend/pkg/server/response/errors.go | 1 + backend/pkg/server/services/auth.go | 19 ++++++++++++++++++- backend/pkg/server/services/roles.go | 4 ++-- backend/pkg/server/services/users.go | 14 +++++++------- backend/pkg/tools/browser.go | 9 ++++++--- 7 files changed, 50 insertions(+), 13 deletions(-) create mode 100644 .claude/settings.local.json diff --git a/.claude/settings.local.json b/.claude/settings.local.json new file mode 100644 index 0000000..b55e75b --- /dev/null +++ b/.claude/settings.local.json @@ -0,0 +1,7 @@ +{ + "permissions": { + "allow": [ + "Bash(go build:*)" + ] + } +} diff --git a/backend/pkg/server/auth/auth_middleware.go b/backend/pkg/server/auth/auth_middleware.go index b713731..de26058 100644 --- a/backend/pkg/server/auth/auth_middleware.go +++ b/backend/pkg/server/auth/auth_middleware.go @@ -5,6 +5,7 @@ import ( "fmt" "slices" "strings" + "time" "pentagi/pkg/server/models" "pentagi/pkg/server/response" @@ -104,6 +105,14 @@ func (p *AuthMiddleware) tryUserCookieAuthentication(c *gin.Context) (authResult return authResultFail, errors.New("no pemissions granted") } + expVal, ok := exp.(int64) + if !ok { + return authResultFail, errors.New("token claim invalid") + } + if time.Now().Unix() > expVal { + return authResultFail, errors.New("session expired") + } + c.Set("prm", prms) c.Set("uid", uid.(uint64)) c.Set("uhash", uhash.(string)) diff --git a/backend/pkg/server/response/errors.go b/backend/pkg/server/response/errors.go index bd63873..1a1820b 100644 --- a/backend/pkg/server/response/errors.go +++ b/backend/pkg/server/response/errors.go @@ -24,6 +24,7 @@ var ErrAuthInvalidAuthorizationNonce = NewHttpError(400, "Auth.InvalidAuthorizat var ErrAuthInvalidCredentials = NewHttpError(401, "Auth.InvalidCredentials", "invalid login or password") var ErrAuthInvalidUserData = NewHttpError(500, "Auth.InvalidUserData", "invalid user data") var ErrAuthInactiveUser = NewHttpError(403, "Auth.InactiveUser", "user is inactive") +var ErrAuthPasswordChangeRequired = NewHttpError(403, "Auth.PasswordChangeRequired", "password change required") var ErrAuthExchangeTokenFail = NewHttpError(403, "Auth.ExchangeTokenFail", "error on exchanging token") var ErrAuthTokenExpired = NewHttpError(403, "Auth.TokenExpired", "token is expired") var ErrAuthVerificationTokenFail = NewHttpError(403, "Auth.VerificationTokenFail", "error on verifying token") diff --git a/backend/pkg/server/services/auth.go b/backend/pkg/server/services/auth.go index a2ac454..6b5f3ae 100644 --- a/backend/pkg/server/services/auth.go +++ b/backend/pkg/server/services/auth.go @@ -124,6 +124,12 @@ func (s *AuthService) AuthLogin(c *gin.Context) { return } + if user.PasswordChangeRequired { + logger.FromContext(c).Warnf("password change required for user '%s'", user.Hash) + response.Error(c, response.ErrAuthPasswordChangeRequired, fmt.Errorf("password change required")) + return + } + var privs []string err := s.db.Table("privileges"). Where("role_id = ?", user.RoleID). @@ -155,6 +161,7 @@ func (s *AuthService) AuthLogin(c *gin.Context) { session.Options(sessions.Options{ HttpOnly: true, Secure: c.Request.TLS != nil, + SameSite: http.SameSiteLaxMode, Path: s.cfg.BaseURL, MaxAge: int(expires), }) @@ -195,6 +202,7 @@ func (s *AuthService) refreshCookie(c *gin.Context, resp *info, privs []string) session.Options(sessions.Options{ HttpOnly: true, Secure: c.Request.TLS != nil, + SameSite: http.SameSiteLaxMode, Path: s.cfg.BaseURL, MaxAge: expires, }) @@ -325,6 +333,12 @@ func (s *AuthService) AuthLoginGetCallback(c *gin.Context) { return } + if queryState := c.Query("state"); queryState != "" && queryState != state.Value { + logger.FromContext(c).Errorf("error matching received state to stored one") + response.Error(c, response.ErrAuthInvalidAuthorizationState, nil) + return + } + stateData, err := s.parseState(c, state.Value) if err != nil { return @@ -546,6 +560,7 @@ func (s *AuthService) authLoginCallback(c *gin.Context, stateData map[string]str session.Options(sessions.Options{ HttpOnly: true, Secure: c.Request.TLS != nil, + SameSite: http.SameSiteLaxMode, Path: s.cfg.BaseURL, MaxAge: expires, }) @@ -597,13 +612,13 @@ func (s *AuthService) parseState(c *gin.Context, state string) (map[string]strin } signatureLen := 32 - stateSignature := stateJSON[:signatureLen] if len(stateJSON) <= signatureLen { logger.FromContext(c).Errorf("error on parsing state from json data") err := fmt.Errorf("unexpected state length") response.Error(c, response.ErrAuthInvalidAuthorizationState, err) return nil, err } + stateSignature := stateJSON[:signatureLen] stateJSON = stateJSON[signatureLen:] mac := hmac.New(sha256.New, s.key) @@ -646,6 +661,7 @@ func (s *AuthService) setCallbackCookie(w http.ResponseWriter, r *http.Request, Value: value, HttpOnly: true, Secure: r.TLS != nil, + SameSite: http.SameSiteLaxMode, Path: path.Join(s.cfg.BaseURL, s.cfg.LoginCallbackURL), MaxAge: maxAge, } @@ -660,6 +676,7 @@ func (s *AuthService) resetSession(c *gin.Context) { session.Options(sessions.Options{ HttpOnly: true, Secure: c.Request.TLS != nil, + SameSite: http.SameSiteLaxMode, Path: s.cfg.BaseURL, MaxAge: -1, }) diff --git a/backend/pkg/server/services/roles.go b/backend/pkg/server/services/roles.go index 7888623..3674524 100644 --- a/backend/pkg/server/services/roles.go +++ b/backend/pkg/server/services/roles.go @@ -63,7 +63,7 @@ func (s *RoleService) GetRoles(c *gin.Context) { rid := c.GetUint64("rid") privs := c.GetStringSlice("prm") scope := func(db *gorm.DB) *gorm.DB { - if !slices.Contains(privs, "roles.view'") { + if !slices.Contains(privs, "roles.view") { return db.Where("role_id = ?", rid) } return db @@ -128,7 +128,7 @@ func (s *RoleService) GetRole(c *gin.Context) { rid := c.GetUint64("rid") privs := c.GetStringSlice("prm") scope := func(db *gorm.DB) *gorm.DB { - if !slices.Contains(privs, "roles.view'") { + if !slices.Contains(privs, "roles.view") { return db.Where("role_id = ?", rid) } return db diff --git a/backend/pkg/server/services/users.go b/backend/pkg/server/services/users.go index 3586a9f..625c575 100644 --- a/backend/pkg/server/services/users.go +++ b/backend/pkg/server/services/users.go @@ -181,7 +181,7 @@ func (s *UserService) GetUsers(c *gin.Context) { uid := c.GetUint64("uid") privs := c.GetStringSlice("prm") scope := func(db *gorm.DB) *gorm.DB { - if !slices.Contains(privs, "users.view'") { + if !slices.Contains(privs, "users.view") { return db.Where("id = ?", uid) } return db @@ -257,7 +257,7 @@ func (s *UserService) GetUser(c *gin.Context) { uhash := c.GetString("uhash") privs := c.GetStringSlice("prm") - if !slices.Contains(privs, "users.view'") && uhash != hash { + if !slices.Contains(privs, "users.view") && uhash != hash { logger.FromContext(c).Errorf("error filtering user role permissions: permission not found") response.Error(c, response.ErrNotPermitted, nil) return @@ -318,7 +318,7 @@ func (s *UserService) CreateUser(c *gin.Context) { rid := c.GetUint64("rid") privs := c.GetStringSlice("prm") - if !slices.Contains(privs, "users.create'") { + if !slices.Contains(privs, "users.create") { logger.FromContext(c).Errorf("error filtering user role permissions: permission not found") response.Error(c, response.ErrNotPermitted, nil) return @@ -423,13 +423,13 @@ func (s *UserService) PatchUser(c *gin.Context) { uhash := c.GetString("uhash") privs := c.GetStringSlice("prm") scope := func(db *gorm.DB) *gorm.DB { - if slices.Contains(privs, "users.edit'") { + if slices.Contains(privs, "users.edit") { return db.Where("hash = ?", hash) } else { return db.Where("hash = ? AND id = ?", hash, uid) } } - if !slices.Contains(privs, "users.edit'") && uhash != hash { + if !slices.Contains(privs, "users.edit") && uhash != hash { logger.FromContext(c).Errorf("error filtering user role permissions: permission not found") response.Error(c, response.ErrNotPermitted, nil) return @@ -510,13 +510,13 @@ func (s *UserService) DeleteUser(c *gin.Context) { uhash := c.GetString("uhash") privs := c.GetStringSlice("prm") scope := func(db *gorm.DB) *gorm.DB { - if slices.Contains(privs, "users.delete'") { + if slices.Contains(privs, "users.delete") { return db.Where("hash = ?", hash) } else { return db.Where("hash = ? AND id = ?", hash, uid) } } - if !slices.Contains(privs, "users.delete'") && uhash != hash { + if !slices.Contains(privs, "users.delete") && uhash != hash { logger.FromContext(c).Errorf("error filtering user role permissions: permission not found") response.Error(c, response.ErrNotPermitted, nil) return diff --git a/backend/pkg/tools/browser.go b/backend/pkg/tools/browser.go index 26f6aee..9bb98ec 100644 --- a/backend/pkg/tools/browser.go +++ b/backend/pkg/tools/browser.go @@ -422,9 +422,12 @@ func (b *browser) getScreenshot(targetURL string) (string, error) { } func (b *browser) callScraper(url string) ([]byte, error) { - client := &http.Client{Transport: &http.Transport{ - TLSClientConfig: &tls.Config{InsecureSkipVerify: true}, - }} + client := &http.Client{ + Timeout: 30 * time.Second, + Transport: &http.Transport{ + TLSClientConfig: &tls.Config{InsecureSkipVerify: true}, + }, + } resp, err := client.Get(url) if err != nil { return nil, fmt.Errorf("failed to fetch data by scraper '%s': %w", url, err) From 6ba22d6e8cebe4813cf16420a88299194ed36713 Mon Sep 17 00:00:00 2001 From: Mriganka Date: Sat, 21 Feb 2026 09:19:09 +0530 Subject: [PATCH 2/3] Reviewer-requested changes: remove server-side password change block and adjust scraper timeout --- backend/pkg/server/response/errors.go | 1 - backend/pkg/server/services/auth.go | 6 ------ backend/pkg/tools/browser.go | 2 +- 3 files changed, 1 insertion(+), 8 deletions(-) diff --git a/backend/pkg/server/response/errors.go b/backend/pkg/server/response/errors.go index 1a1820b..bd63873 100644 --- a/backend/pkg/server/response/errors.go +++ b/backend/pkg/server/response/errors.go @@ -24,7 +24,6 @@ var ErrAuthInvalidAuthorizationNonce = NewHttpError(400, "Auth.InvalidAuthorizat var ErrAuthInvalidCredentials = NewHttpError(401, "Auth.InvalidCredentials", "invalid login or password") var ErrAuthInvalidUserData = NewHttpError(500, "Auth.InvalidUserData", "invalid user data") var ErrAuthInactiveUser = NewHttpError(403, "Auth.InactiveUser", "user is inactive") -var ErrAuthPasswordChangeRequired = NewHttpError(403, "Auth.PasswordChangeRequired", "password change required") var ErrAuthExchangeTokenFail = NewHttpError(403, "Auth.ExchangeTokenFail", "error on exchanging token") var ErrAuthTokenExpired = NewHttpError(403, "Auth.TokenExpired", "token is expired") var ErrAuthVerificationTokenFail = NewHttpError(403, "Auth.VerificationTokenFail", "error on verifying token") diff --git a/backend/pkg/server/services/auth.go b/backend/pkg/server/services/auth.go index 6b5f3ae..53a27d5 100644 --- a/backend/pkg/server/services/auth.go +++ b/backend/pkg/server/services/auth.go @@ -124,12 +124,6 @@ func (s *AuthService) AuthLogin(c *gin.Context) { return } - if user.PasswordChangeRequired { - logger.FromContext(c).Warnf("password change required for user '%s'", user.Hash) - response.Error(c, response.ErrAuthPasswordChangeRequired, fmt.Errorf("password change required")) - return - } - var privs []string err := s.db.Table("privileges"). Where("role_id = ?", user.RoleID). diff --git a/backend/pkg/tools/browser.go b/backend/pkg/tools/browser.go index 9bb98ec..b2d0bcb 100644 --- a/backend/pkg/tools/browser.go +++ b/backend/pkg/tools/browser.go @@ -423,7 +423,7 @@ func (b *browser) getScreenshot(targetURL string) (string, error) { func (b *browser) callScraper(url string) ([]byte, error) { client := &http.Client{ - Timeout: 30 * time.Second, + Timeout: 65 * time.Second, Transport: &http.Transport{ TLSClientConfig: &tls.Config{InsecureSkipVerify: true}, }, From ec4a24d7af7728f8cd7bcdfbb4ffa776d0a9e363 Mon Sep 17 00:00:00 2001 From: Mriganka Date: Sat, 21 Feb 2026 09:20:51 +0530 Subject: [PATCH 3/3] Remove .claude folder and add to .gitignore --- .claude/settings.local.json | 7 ------- .gitignore | 2 ++ 2 files changed, 2 insertions(+), 7 deletions(-) delete mode 100644 .claude/settings.local.json diff --git a/.claude/settings.local.json b/.claude/settings.local.json deleted file mode 100644 index b55e75b..0000000 --- a/.claude/settings.local.json +++ /dev/null @@ -1,7 +0,0 @@ -{ - "permissions": { - "allow": [ - "Bash(go build:*)" - ] - } -} diff --git a/.gitignore b/.gitignore index c295232..318c9f1 100644 --- a/.gitignore +++ b/.gitignore @@ -22,3 +22,5 @@ build/* data/* .bak/* !.gitkeep +.claude/ +