Skip to content

Commit 5ad21df

Browse files
sallyomclaude
andcommitted
fix: Correct RBAC check ordering in PutSessionWorkspaceFile to prevent session enumeration
Moves session existence check to immediately after RBAC validation, before service lookup. This prevents unauthorized users from enumerating valid session names by observing different error responses. Security improvement: - RBAC check first (line 2568-2589) - Session existence check second (line 2591-2602) - Service lookup third (line 2604-2620) This matches the pattern already implemented in DeleteSessionWorkspaceFile and follows the security standards defined in CLAUDE.md. Fixes the issue identified in PR review at sessions.go:2578-2591. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
1 parent bad8359 commit 5ad21df

File tree

1 file changed

+15
-11
lines changed

1 file changed

+15
-11
lines changed

components/backend/handlers/sessions.go

Lines changed: 15 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -2566,6 +2566,7 @@ func PutSessionWorkspaceFile(c *gin.Context) {
25662566
}
25672567

25682568
// RBAC check: verify user has update permission on agenticsessions (file operations modify session state)
2569+
// IMPORTANT: RBAC check MUST happen BEFORE checking session existence to prevent enumeration attacks
25692570
ssar := &authzv1.SelfSubjectAccessReview{
25702571
Spec: authzv1.SelfSubjectAccessReviewSpec{
25712572
ResourceAttributes: &authzv1.ResourceAttributes{
@@ -2587,6 +2588,19 @@ func PutSessionWorkspaceFile(c *gin.Context) {
25872588
return
25882589
}
25892590

2591+
// Verify session exists using reqDyn AFTER RBAC check
2592+
// This prevents enumeration attacks - unauthorized users get same "Forbidden" response
2593+
gvr := GetAgenticSessionV1Alpha1Resource()
2594+
item, err := reqDyn.Resource(gvr).Namespace(project).Get(c.Request.Context(), session, v1.GetOptions{})
2595+
if err != nil {
2596+
if errors.IsNotFound(err) {
2597+
c.JSON(http.StatusNotFound, gin.H{"error": "Session not found"})
2598+
return
2599+
}
2600+
c.JSON(http.StatusInternalServerError, gin.H{"error": "Failed to get session"})
2601+
return
2602+
}
2603+
25902604
// Try temp service first (for completed sessions), then regular service
25912605
serviceName := fmt.Sprintf("temp-content-%s", session)
25922606
serviceFound := false
@@ -2606,18 +2620,8 @@ func PutSessionWorkspaceFile(c *gin.Context) {
26062620
}
26072621

26082622
// If no service exists, request temp content pod and return accepted status
2609-
// reqDyn is guaranteed non-nil after auth check above
2623+
// We already have the session item from the existence check above
26102624
if !serviceFound {
2611-
gvr := GetAgenticSessionV1Alpha1Resource()
2612-
item, err := reqDyn.Resource(gvr).Namespace(project).Get(c.Request.Context(), session, v1.GetOptions{})
2613-
if err != nil {
2614-
if errors.IsNotFound(err) {
2615-
c.JSON(http.StatusNotFound, gin.H{"error": "Session not found"})
2616-
return
2617-
}
2618-
c.JSON(http.StatusInternalServerError, gin.H{"error": "Failed to get session"})
2619-
return
2620-
}
26212625

26222626
// Check if temp content was already requested (avoid duplicate pod creation)
26232627
annotations := item.GetAnnotations()

0 commit comments

Comments
 (0)