[PR #104] [MERGED] Security and Bug Fixes #158

New Issue

2026-06-06T22:09:28-04:00

yindo commented

2026-06-06 22:09:28 -04:00

📋 Pull Request Information

Original PR: https://github.com/vxcontrol/pentagi/pull/104
Author: @mrigankad
Created: 2/20/2026
Status: ✅ Merged
Merged: 2/20/2026
Merged by: @asdek

Base: master ← Head: fix/security-and-bugs

📝 Commits (3)

8c1f5e7 Security and Bug Fixes
6ba22d6 Reviewer-requested changes: remove server-side password change block and adjust scraper timeout
ec4a24d Remove .claude folder and add to .gitignore

📊 Changes

6 files changed (+38 additions, -13 deletions)

View changed files

📝 .gitignore (+2 -0)
📝 backend/pkg/server/auth/auth_middleware.go (+9 -0)
📝 backend/pkg/server/services/auth.go (+12 -1)
📝 backend/pkg/server/services/roles.go (+2 -2)
📝 backend/pkg/server/services/users.go (+7 -7)
📝 backend/pkg/tools/browser.go (+6 -3)

📄 Description

PR Summary: 🔒 Security Hardening and Critical Bug Fixes

Overview

This PR addresses several security vulnerabilities and critical bugs identified in the authentication service, session management, and external tool integrations. The fixes range from preventing DoS attacks to correcting broken access control logic (ACL).

Key Changes

🛡️ Security Fixes

Issue 8: DoS Prevention in parseState()
- Added a strict length check for stateJSON before attempting to slice the signature. This prevents a runtime panic when processing malformed or intentionally short input.
Issue 5: CSRF Protection in OAuth Callbacks
- Updated AuthLoginGetCallback to validate the state parameter against the stored session cookie, closing a CSRF gap in the GET handler.
Issue 7: Cookie Security
- Enforced SameSite=Lax for all session and callback cookies to mitigate CSRF risks.
Issue 1: Password Policy Enforcement
- Added server-side validation to block login if a user has PasswordChangeRequired set to true, ensuring the policy is enforced before any session is granted.

🛠️ Critical Bug Fixes

Issue 11: Broken ACL Correction
- Removed trailing typos (single quotes) in all 7 privilege strings (e.g., "users.view'" → "users.view"). These typos previously caused all permission checks to fail silently.
Issue 6: Session Expiry Enforcement
- Updated the authentication middleware to verify the exp claim. Sessions are now correctly invalidated as soon as they expire, even outside the /info route.
Issue 9: HTTP Client Timeout
- Added a 30-second timeout to the browser tool's scraper client to prevent indefinite hangs that could lead to resource exhaustion.

Impact

Security: Strengthened mitigation against DoS, CSRF, and unauthorized access.
Stability: Fixed a panic in auth state parsing and prevented hung processes in the browser tool.
Reliability: Restored functional RBAC/ACL checks which were broken due to string constants.

Checklist

Malformed short input for state does not cause panic.
CSRF state is validated for both GET and POST callbacks.
Cookies include the SameSite attribute.
Privilege strings match the database entries exactly.
Expired session cookies return HTTP 403.
Browser tool requests timeout correctly.

_{🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.}

## 📋 Pull Request Information **Original PR:** https://github.com/vxcontrol/pentagi/pull/104 **Author:** [@mrigankad](https://github.com/mrigankad) **Created:** 2/20/2026 **Status:** ✅ Merged **Merged:** 2/20/2026 **Merged by:** [@asdek](https://github.com/asdek) **Base:** `master` ← **Head:** `fix/security-and-bugs` --- ### 📝 Commits (3) - [`8c1f5e7`](https://github.com/vxcontrol/pentagi/commit/8c1f5e7d4c4c9f053bb1b30bc6216d004d8032f1) Security and Bug Fixes - [`6ba22d6`](https://github.com/vxcontrol/pentagi/commit/6ba22d6e8cebe4813cf16420a88299194ed36713) Reviewer-requested changes: remove server-side password change block and adjust scraper timeout - [`ec4a24d`](https://github.com/vxcontrol/pentagi/commit/ec4a24d7af7728f8cd7bcdfbb4ffa776d0a9e363) Remove .claude folder and add to .gitignore ### 📊 Changes **6 files changed** (+38 additions, -13 deletions) <details> <summary>View changed files</summary> 📝 `.gitignore` (+2 -0) 📝 `backend/pkg/server/auth/auth_middleware.go` (+9 -0) 📝 `backend/pkg/server/services/auth.go` (+12 -1) 📝 `backend/pkg/server/services/roles.go` (+2 -2) 📝 `backend/pkg/server/services/users.go` (+7 -7) 📝 `backend/pkg/tools/browser.go` (+6 -3) </details> ### 📄 Description # PR Summary: 🔒 Security Hardening and Critical Bug Fixes ## Overview This PR addresses several security vulnerabilities and critical bugs identified in the authentication service, session management, and external tool integrations. The fixes range from preventing DoS attacks to correcting broken access control logic (ACL). ## Key Changes ### 🛡️ Security Fixes - **Issue 8: DoS Prevention in [parseState()](cci:1://file:///c:/Users/mriga_ijtdono/Desktop/Pentagi/backend/pkg/server/services/auth.go:603:0-655:1)** - Added a strict length check for `stateJSON` before attempting to slice the signature. This prevents a runtime panic when processing malformed or intentionally short input. - **Issue 5: CSRF Protection in OAuth Callbacks** - Updated [AuthLoginGetCallback](cci:1://file:///c:/Users/mriga_ijtdono/Desktop/Pentagi/backend/pkg/server/services/auth.go:309:0-347:1) to validate the `state` parameter against the stored session cookie, closing a CSRF gap in the GET handler. - **Issue 7: Cookie Security** - Enforced `SameSite=Lax` for all session and callback cookies to mitigate CSRF risks. - **Issue 1: Password Policy Enforcement** - Added server-side validation to block login if a user has `PasswordChangeRequired` set to `true`, ensuring the policy is enforced before any session is granted. ### 🛠️ Critical Bug Fixes - **Issue 11: Broken ACL Correction** - Removed trailing typos (single quotes) in all 7 privilege strings (e.g., `"users.view'"` → `"users.view"`). These typos previously caused all permission checks to fail silently. - **Issue 6: Session Expiry Enforcement** - Updated the authentication middleware to verify the `exp` claim. Sessions are now correctly invalidated as soon as they expire, even outside the `/info` route. - **Issue 9: HTTP Client Timeout** - Added a 30-second timeout to the browser tool's scraper client to prevent indefinite hangs that could lead to resource exhaustion. ## Impact - **Security**: Strengthened mitigation against DoS, CSRF, and unauthorized access. - **Stability**: Fixed a panic in auth state parsing and prevented hung processes in the browser tool. - **Reliability**: Restored functional RBAC/ACL checks which were broken due to string constants. ## Checklist - [x] Malformed short input for state does not cause panic. - [x] CSRF state is validated for both GET and POST callbacks. - [x] Cookies include the `SameSite` attribute. - [x] Privilege strings match the database entries exactly. - [x] Expired session cookies return HTTP 403. - [x] Browser tool requests timeout correctly. --- <sub>🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.</sub>

yindo added the pull-request label 2026-06-06 22:09:28 -04:00

yindo closed this issue

2026-06-06 22:09:28 -04:00

yindo referenced this issue

2026-06-06 22:09:40 -04:00

[PR #158] [CLOSED] fix(browser): treat screenshot failure as non-critical warning #198

Sign in to join this conversation.

1 Participants

Notifications

Due Date

No due date set.

Dependencies

No dependencies set.

Reference: vxcontrol/pentagi#158