[PR #131] [CLOSED] fix: replace unsafe type assertions in session cookie middleware #179

New Issue

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

yindo commented

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

📋 Pull Request Information

Original PR: https://github.com/vxcontrol/pentagi/pull/131
Author: @mason5052
Created: 2/22/2026
Status: ❌ Closed

Base: master ← Head: fix/auth-middleware-unsafe-type-assertions

📝 Commits (1)

d26209b fix: replace unsafe type assertions in session cookie middleware

📊 Changes

1 file changed (+31 additions, -8 deletions)

View changed files

📝 backend/pkg/server/auth/auth_middleware.go (+31 -8)

📄 Description

Description of the Change

Problem

session.Get() returns interface{} for each session claim. The middleware correctly validates that all required claims are non-nil (line 102) and uses the two-value assertion form for prm and exp (lines 108-117), but then performs bare, unchecked type assertions on six remaining claims:

// BEFORE - panics if session data has wrong type
userID := uid.(uint64)        // line 123
sessionHash := uhash.(string) // line 124
// ...
c.Set("rid", rid.(uint64))    // line 149
c.Set("exp", exp.(int64))     // line 150
c.Set("gtm", gtm.(int64))     // line 151
c.Set("tid", tid.(string))    // line 152
c.Set("uname", uname.(string))// line 153

A crafted or corrupted session cookie containing a claim with the wrong underlying type (e.g., uid stored as string instead of uint64) causes a runtime panic. Gin's recovery middleware catches the panic and returns a 500, but the request fails and the panic is logged as an unhandled error.

Solution

Replace all bare type assertions with the two-value form and return authResultFail on type mismatch, consistent with the pattern already established in the same function for prm and exp:

// AFTER - returns 403 (authResultFail / ErrAuthRequired) instead of panicking
userID, ok := uid.(uint64)
if !ok {
    return authResultFail, errors.New("uid claim has invalid type")
}

Also fixes a pre-existing typo: "no pemissions granted" -> "no permissions granted".

Ref: #101

Type of Change

Bug fix (non-breaking change which fixes an issue)
Security update

Areas Affected

Core Services (Frontend UI/Backend API)

Testing and Verification

Test Steps

Normal login flow continues to work (all claims have correct types under normal operation)
Malformed session cookie with wrong-typed claim now returns 403 (authResultFail) instead of 500 panic

Security Considerations

Prevents a DoS vector: an attacker who can craft a session cookie with wrong-typed claims could previously trigger panics. Now returns a clean authResultFail (HTTP 403) instead.
No change to the authentication logic itself - only error handling for malformed session data is improved.

Checklist

Code Quality

My code follows the project's coding standards
All new and existing tests pass
I have run go fmt and go vet (for Go code)

Security

I have considered security implications
Changes maintain or improve the security model

Compatibility

Changes are backward compatible

_{🔄 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/131 **Author:** [@mason5052](https://github.com/mason5052) **Created:** 2/22/2026 **Status:** ❌ Closed **Base:** `master` ← **Head:** `fix/auth-middleware-unsafe-type-assertions` --- ### 📝 Commits (1) - [`d26209b`](https://github.com/vxcontrol/pentagi/commit/d26209bb3033b605df74f2f68542cebf859e3844) fix: replace unsafe type assertions in session cookie middleware ### 📊 Changes **1 file changed** (+31 additions, -8 deletions) <details> <summary>View changed files</summary> 📝 `backend/pkg/server/auth/auth_middleware.go` (+31 -8) </details> ### 📄 Description ## Description of the Change ### Problem `session.Get()` returns `interface{}` for each session claim. The middleware correctly validates that all required claims are non-nil (line 102) and uses the two-value assertion form for `prm` and `exp` (lines 108-117), but then performs bare, unchecked type assertions on six remaining claims: ```go // BEFORE - panics if session data has wrong type userID := uid.(uint64) // line 123 sessionHash := uhash.(string) // line 124 // ... c.Set("rid", rid.(uint64)) // line 149 c.Set("exp", exp.(int64)) // line 150 c.Set("gtm", gtm.(int64)) // line 151 c.Set("tid", tid.(string)) // line 152 c.Set("uname", uname.(string))// line 153 ``` A crafted or corrupted session cookie containing a claim with the wrong underlying type (e.g., `uid` stored as `string` instead of `uint64`) causes a runtime panic. Gin's recovery middleware catches the panic and returns a 500, but the request fails and the panic is logged as an unhandled error. ### Solution Replace all bare type assertions with the two-value form and return `authResultFail` on type mismatch, consistent with the pattern already established in the same function for `prm` and `exp`: ```go // AFTER - returns 403 (authResultFail / ErrAuthRequired) instead of panicking userID, ok := uid.(uint64) if !ok { return authResultFail, errors.New("uid claim has invalid type") } ``` Also fixes a pre-existing typo: `"no pemissions granted"` -> `"no permissions granted"`. Ref: #101 ### Type of Change - [x] Bug fix (non-breaking change which fixes an issue) - [x] Security update ### Areas Affected - [x] Core Services (Frontend UI/Backend API) ### Testing and Verification #### Test Steps 1. Normal login flow continues to work (all claims have correct types under normal operation) 2. Malformed session cookie with wrong-typed claim now returns 403 (authResultFail) instead of 500 panic ### Security Considerations - Prevents a DoS vector: an attacker who can craft a session cookie with wrong-typed claims could previously trigger panics. Now returns a clean `authResultFail` (HTTP 403) instead. - No change to the authentication logic itself - only error handling for malformed session data is improved. ### Checklist #### Code Quality - [x] My code follows the project's coding standards - [x] All new and existing tests pass - [x] I have run `go fmt` and `go vet` (for Go code) #### Security - [x] I have considered security implications - [x] Changes maintain or improve the security model #### Compatibility - [x] Changes are backward compatible --- <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:35 -04:00

yindo closed this issue

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

yindo referenced this issue

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

[PR #179] [MERGED] fix: isolate detached command context from parent cancellation #216

yindo referenced this issue

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

[PR #181] [CLOSED] test: add unit tests for terminal ExecCommand detach behavior #218

yindo referenced this issue

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

[PR #206] [MERGED] Agent Supervision System and Open Source Model Enhancements #235