RBAC getRequiredRoles bypasses wildcard when exact match lacks method roles #685

New issue

Closed

opened 2026-03-28 04:27:25 +00:00 by mfreeman451 · 1 comment

mfreeman451 commented

2026-03-28 04:27:25 +00:00

Owner

Imported from GitHub.

Original GitHub issue: #2147
Original author: @mfreeman451
Original URL: https://github.com/carverauto/serviceradar/issues/2147
Original created: 2025-12-16T05:17:02Z

Summary

Context: The getRequiredRoles function in pkg/core/auth/rbac.go determines which roles are required to access HTTP routes based on the RBAC configuration, supporting both exact path matches and wildcard patterns.
Bug: When a route has both an exact path match with method-specific roles AND a wildcard pattern protection, the exact match takes precedence and bypasses wildcard protection for methods not explicitly defined in the exact match.
Actual vs. expected: If /api/admin/* requires ["admin"] role and /api/admin/users has method-specific rules {"POST": ["superadmin"]}, a GET request to /api/admin/users returns no required roles (allowing unrestricted access) instead of falling back to the wildcard's ["admin"] requirement.
Impact: This creates a security vulnerability where protected admin routes can be accessed without authentication if someone adds method-specific rules for only some HTTP methods, inadvertently opening unrestricted access for other methods.

Code with bug

// getRequiredRoles determines which roles are required for a given route and method
func getRequiredRoles(path, method string, routeProtection map[string]interface{}) []string {
	if routeProtection == nil {
		return []string{}
	}

	// Check for exact match first
	if protection, exists := routeProtection[path]; exists {
		return parseProtection(protection, method)  // <-- BUG 🔴 Returns early even if method not found, bypassing wildcard
	}

	// Check for wildcard matches
	for pattern, protection := range routeProtection {
		if matchesPattern(path, pattern) {
			return parseProtection(protection, method)
		}
	}

	return []string{}
}

Evidence

Example

Consider this RBAC configuration (from the actual config files in this codebase):

{
  "route_protection": {
    "/api/admin/*": ["admin"],
    "/api/admin/users": {
      "POST": ["superadmin"]
    }
  }
}

Step-by-step execution for GET /api/admin/users:

getRequiredRoles("/api/admin/users", "GET", routeProtection) is called
Line 78: Exact match found for /api/admin/users
Line 79: parseProtection({"POST": ["superadmin"]}, "GET") is called
parseProtection checks the map for "GET" key at line 109
"GET" key doesn't exist in the map
Line 112: Returns []string{}
Line 79: getRequiredRoles returns []string{}
Result: No roles required, access is granted to ANY user (even unauthenticated ones if they bypass the authentication middleware)

Expected behavior:

The function should check the wildcard pattern /api/admin/*
Return ["admin"] as required roles
Deny access to users without the admin role

Failing test

Test script

/*
 * Copyright 2025 Carver Automation Corporation.
 *
 * Licensed under the Apache License, Version 2.0 (the "License");
 * you may not use this file except in compliance with the License.
 * You may obtain a copy of the License at
 *
 *     http://www.apache.org/licenses/LICENSE-2.0
 *
 * Unless required by applicable law or agreed to in writing, software
 * distributed under the License is distributed on an "AS IS" BASIS,
 * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
 * See the License for the specific language governing permissions and
 * limitations under the License.
 */

package auth

import (
	"testing"

	"github.com/stretchr/testify/assert"
)

// TestGetRequiredRoles_ExactMatchBypassesWildcard demonstrates a security bug
// where an exact path match with method-specific roles bypasses wildcard protection
func TestGetRequiredRoles_ExactMatchBypassesWildcard(t *testing.T) {
	// Scenario: Admin routes are protected with wildcard "/api/admin/*": ["admin"]
	// But a specific route "/api/admin/users" has method-specific protection for POST only
	// Bug: GET requests to "/api/admin/users" will bypass the wildcard protection
	routeProtection := map[string]interface{}{
		"/api/admin/*": []string{"admin"}, // All admin routes require "admin" role
		"/api/admin/users": map[string]interface{}{ // Specific route with method-specific rules
			"POST": []string{"superadmin"}, // POST requires superadmin
		},
	}

	// Test GET request to /api/admin/users
	// Expected: Should require "admin" role (from wildcard)
	// Actual: Returns empty array (no protection)
	roles := getRequiredRoles("/api/admin/users", "GET", routeProtection)

	// This assertion will FAIL, demonstrating the bug
	assert.NotEmpty(t, roles, "GET /api/admin/users should require admin role from wildcard pattern, but got no protection")
	assert.Contains(t, roles, "admin", "Should inherit admin requirement from /api/admin/* wildcard")
}

// TestGetRequiredRoles_ExactMatchEmptyBypassesWildcard is another variant
func TestGetRequiredRoles_ExactMatchEmptyBypassesWildcard(t *testing.T) {
	// Scenario: Similar to above but with empty method map
	routeProtection := map[string]interface{}{
		"/api/admin/*": []string{"admin"},
		"/api/admin/config": map[string]interface{}{
			"DELETE": []string{"superadmin"},
		},
	}

	// Test GET request - the exact match exists but has no GET method defined
	roles := getRequiredRoles("/api/admin/config", "GET", routeProtection)

	// Bug: Returns empty, allowing unrestricted access despite wildcard protection
	assert.NotEmpty(t, roles, "GET /api/admin/config should require admin role from wildcard")
}

// TestGetRequiredRoles_WildcardWorksWithoutExactMatch shows the expected behavior
func TestGetRequiredRoles_WildcardWorksWithoutExactMatch(t *testing.T) {
	routeProtection := map[string]interface{}{
		"/api/admin/*": []string{"admin"},
	}

	// Without an exact match, wildcard works correctly
	roles := getRequiredRoles("/api/admin/users", "GET", routeProtection)
	assert.Equal(t, []string{"admin"}, roles, "Wildcard should work when no exact match exists")
}

Test output

=== RUN   TestGetRequiredRoles_ExactMatchBypassesWildcard
    rbac_bug_test.go:44:
        	Error Trace:	/home/user/serviceradar/pkg/core/auth/rbac_bug_test.go:44
        	Error:      	Should NOT be empty, but was []
        	Test:       	TestGetRequiredRoles_ExactMatchBypassesWildcard
        	Messages:   	GET /api/admin/users should require admin role from wildcard pattern, but got no protection
    rbac_bug_test.go:45:
        	Error Trace:	/home/user/serviceradar/pkg/core/auth/rbac_bug_test.go:45
        	Error:      	[]string{} does not contain "admin"
        	Test:       	TestGetRequiredRoles_ExactMatchBypassesWildcard
        	Messages:   	Should inherit admin requirement from /api/admin/* wildcard
--- FAIL: TestGetRequiredRoles_ExactMatchBypassesWildcard (0.00s)
=== RUN   TestGetRequiredRoles_ExactMatchEmptyBypassesWildcard
    rbac_bug_test.go:62:
        	Error Trace:	/home/user/serviceradar/pkg/core/auth/rbac_bug_test.go:62
        	Error:      	Should NOT be empty, but was []
        	Test:       	TestGetRequiredRoles_ExactMatchEmptyBypassesWildcard
        	Messages:   	GET /api/admin/config should require admin role from wildcard
--- FAIL: TestGetRequiredRoles_ExactMatchEmptyBypassesWildcard (0.00s)
=== RUN   TestGetRequiredRoles_WildcardWorksWithoutExactMatch
--- PASS: TestGetRequiredRoles_WildcardWorksWithoutExactMatch (0.00s)
FAIL
FAIL	github.com/carverauto/serviceradar/pkg/core/auth	0.005s
FAIL

Full context

The RouteProtectionMiddleware is applied to all protected API routes in the HTTP server. When a request comes in, it:

Extracts the authenticated user from the request context
Calls getRequiredRoles to determine which roles are needed for the requested path and HTTP method
If getRequiredRoles returns an empty array, the middleware interprets this as "no protection required" and allows the request to proceed without any role checks
Otherwise, it verifies that the user has at least one of the required roles

The bug occurs in step 2-3: when an exact path match exists but doesn't define roles for the specific HTTP method being used, getRequiredRoles returns an empty array instead of falling back to wildcard pattern matches.

This is particularly dangerous because:

The actual configuration files in this codebase (e.g., packaging/core/config/core.json, helm/serviceradar/files/serviceradar-config.yaml) use wildcard patterns like /api/admin/* to protect entire sections of the API
Developers might add method-specific rules for certain endpoints (like requiring superadmin for POST operations) without realizing this creates a security hole for other HTTP methods on that same endpoint
The middleware treats an empty roles array as "allow all" rather than "deny all"

The bug is in pkg/core/auth/rbac.go:72-90, specifically the early return at line 79 that doesn't check if the returned roles array is empty before bypassing the wildcard pattern checks.

Why has this bug gone undetected?

This bug has gone undetected for several reasons:

No existing tests for the RBAC route protection logic: The only test file in pkg/core/auth/ is auth_test.go, which tests JWT authentication but not the route protection middleware or getRequiredRoles function.
Limited production usage of method-specific rules: Based on the configuration files in the codebase, most route protection uses simple role arrays rather than method-specific maps. The bug only manifests when you have BOTH a wildcard pattern AND an exact match with method-specific rules.
Authentication middleware still active: Even though the route protection is bypassed, the authentication middleware (authenticationMiddleware) still runs before the RBAC middleware (see pkg/core/api/server.go:822-827). This means completely unauthenticated requests are still blocked - only authenticated users without the required roles can exploit this bug.
Common configurations don't trigger the bug: Looking at the example configs, most routes either:
- Use only wildcard patterns (works correctly)
- Use only exact matches with method-specific rules (works, just doesn't inherit from wildcards)
- Use exact matches with role arrays instead of method maps (works correctly)
The bug requires specific configuration pattern: You need someone to:
- First define a wildcard protection (e.g., /api/admin/*: ["admin"])
- Then add a more specific rule with method-specific roles (e.g., /api/admin/users: {"POST": ["superadmin"]})
- Then make a request with a different HTTP method (e.g., GET)

This specific sequence is uncommon but not unrealistic - a developer might want to add stricter requirements for certain operations (POST/DELETE) while assuming the wildcard still protects read operations (GET).

Recommended fix

The function should only return early from the exact match if it actually found roles for the requested method. If the exact match doesn't specify roles for the method, it should fall through to check wildcard patterns:

func getRequiredRoles(path, method string, routeProtection map[string]interface{}) []string {
	if routeProtection == nil {
		return []string{}
	}

	// Check for exact match first
	if protection, exists := routeProtection[path]; exists {
		roles := parseProtection(protection, method)
		if len(roles) > 0 {  // <-- FIX 🟢 Only return if we found roles
			return roles
		}
		// Otherwise fall through to check wildcards
	}

	// Check for wildcard matches
	for pattern, protection := range routeProtection {
		if matchesPattern(path, pattern) {
			return parseProtection(protection, method)
		}
	}

	return []string{}
}

This ensures that method-specific rules can override wildcard patterns when they specify roles for a method, but wildcard patterns still provide protection when the method-specific rule doesn't define roles for the requested HTTP method.

Imported from GitHub. Original GitHub issue: #2147 Original author: @mfreeman451 Original URL: https://github.com/carverauto/serviceradar/issues/2147 Original created: 2025-12-16T05:17:02Z --- # Summary - **Context**: The `getRequiredRoles` function in `pkg/core/auth/rbac.go` determines which roles are required to access HTTP routes based on the RBAC configuration, supporting both exact path matches and wildcard patterns. - **Bug**: When a route has both an exact path match with method-specific roles AND a wildcard pattern protection, the exact match takes precedence and bypasses wildcard protection for methods not explicitly defined in the exact match. - **Actual vs. expected**: If `/api/admin/*` requires `["admin"]` role and `/api/admin/users` has method-specific rules `{"POST": ["superadmin"]}`, a GET request to `/api/admin/users` returns no required roles (allowing unrestricted access) instead of falling back to the wildcard's `["admin"]` requirement. - **Impact**: This creates a security vulnerability where protected admin routes can be accessed without authentication if someone adds method-specific rules for only some HTTP methods, inadvertently opening unrestricted access for other methods. # Code with bug ```go // getRequiredRoles determines which roles are required for a given route and method func getRequiredRoles(path, method string, routeProtection map[string]interface{}) []string { if routeProtection == nil { return []string{} } // Check for exact match first if protection, exists := routeProtection[path]; exists { return parseProtection(protection, method) // <-- BUG 🔴 Returns early even if method not found, bypassing wildcard } // Check for wildcard matches for pattern, protection := range routeProtection { if matchesPattern(path, pattern) { return parseProtection(protection, method) } } return []string{} } ``` # Evidence ## Example Consider this RBAC configuration (from the actual config files in this codebase): ```json { "route_protection": { "/api/admin/*": ["admin"], "/api/admin/users": { "POST": ["superadmin"] } } } ``` **Step-by-step execution for `GET /api/admin/users`:** 1. `getRequiredRoles("/api/admin/users", "GET", routeProtection)` is called 2. Line 78: Exact match found for `/api/admin/users` 3. Line 79: `parseProtection({"POST": ["superadmin"]}, "GET")` is called 4. `parseProtection` checks the map for `"GET"` key at line 109 5. `"GET"` key doesn't exist in the map 6. Line 112: Returns `[]string{}` 7. Line 79: `getRequiredRoles` returns `[]string{}` 8. **Result**: No roles required, access is granted to ANY user (even unauthenticated ones if they bypass the authentication middleware) **Expected behavior:** - The function should check the wildcard pattern `/api/admin/*` - Return `["admin"]` as required roles - Deny access to users without the `admin` role ## Failing test ### Test script ```go /* * Copyright 2025 Carver Automation Corporation. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. * You may obtain a copy of the License at * * http://www.apache.org/licenses/LICENSE-2.0 * * Unless required by applicable law or agreed to in writing, software * distributed under the License is distributed on an "AS IS" BASIS, * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. * See the License for the specific language governing permissions and * limitations under the License. */ package auth import ( "testing" "github.com/stretchr/testify/assert" ) // TestGetRequiredRoles_ExactMatchBypassesWildcard demonstrates a security bug // where an exact path match with method-specific roles bypasses wildcard protection func TestGetRequiredRoles_ExactMatchBypassesWildcard(t *testing.T) { // Scenario: Admin routes are protected with wildcard "/api/admin/*": ["admin"] // But a specific route "/api/admin/users" has method-specific protection for POST only // Bug: GET requests to "/api/admin/users" will bypass the wildcard protection routeProtection := map[string]interface{}{ "/api/admin/*": []string{"admin"}, // All admin routes require "admin" role "/api/admin/users": map[string]interface{}{ // Specific route with method-specific rules "POST": []string{"superadmin"}, // POST requires superadmin }, } // Test GET request to /api/admin/users // Expected: Should require "admin" role (from wildcard) // Actual: Returns empty array (no protection) roles := getRequiredRoles("/api/admin/users", "GET", routeProtection) // This assertion will FAIL, demonstrating the bug assert.NotEmpty(t, roles, "GET /api/admin/users should require admin role from wildcard pattern, but got no protection") assert.Contains(t, roles, "admin", "Should inherit admin requirement from /api/admin/* wildcard") } // TestGetRequiredRoles_ExactMatchEmptyBypassesWildcard is another variant func TestGetRequiredRoles_ExactMatchEmptyBypassesWildcard(t *testing.T) { // Scenario: Similar to above but with empty method map routeProtection := map[string]interface{}{ "/api/admin/*": []string{"admin"}, "/api/admin/config": map[string]interface{}{ "DELETE": []string{"superadmin"}, }, } // Test GET request - the exact match exists but has no GET method defined roles := getRequiredRoles("/api/admin/config", "GET", routeProtection) // Bug: Returns empty, allowing unrestricted access despite wildcard protection assert.NotEmpty(t, roles, "GET /api/admin/config should require admin role from wildcard") } // TestGetRequiredRoles_WildcardWorksWithoutExactMatch shows the expected behavior func TestGetRequiredRoles_WildcardWorksWithoutExactMatch(t *testing.T) { routeProtection := map[string]interface{}{ "/api/admin/*": []string{"admin"}, } // Without an exact match, wildcard works correctly roles := getRequiredRoles("/api/admin/users", "GET", routeProtection) assert.Equal(t, []string{"admin"}, roles, "Wildcard should work when no exact match exists") } ``` ### Test output ``` === RUN TestGetRequiredRoles_ExactMatchBypassesWildcard rbac_bug_test.go:44: Error Trace: /home/user/serviceradar/pkg/core/auth/rbac_bug_test.go:44 Error: Should NOT be empty, but was [] Test: TestGetRequiredRoles_ExactMatchBypassesWildcard Messages: GET /api/admin/users should require admin role from wildcard pattern, but got no protection rbac_bug_test.go:45: Error Trace: /home/user/serviceradar/pkg/core/auth/rbac_bug_test.go:45 Error: []string{} does not contain "admin" Test: TestGetRequiredRoles_ExactMatchBypassesWildcard Messages: Should inherit admin requirement from /api/admin/* wildcard --- FAIL: TestGetRequiredRoles_ExactMatchBypassesWildcard (0.00s) === RUN TestGetRequiredRoles_ExactMatchEmptyBypassesWildcard rbac_bug_test.go:62: Error Trace: /home/user/serviceradar/pkg/core/auth/rbac_bug_test.go:62 Error: Should NOT be empty, but was [] Test: TestGetRequiredRoles_ExactMatchEmptyBypassesWildcard Messages: GET /api/admin/config should require admin role from wildcard --- FAIL: TestGetRequiredRoles_ExactMatchEmptyBypassesWildcard (0.00s) === RUN TestGetRequiredRoles_WildcardWorksWithoutExactMatch --- PASS: TestGetRequiredRoles_WildcardWorksWithoutExactMatch (0.00s) FAIL FAIL github.com/carverauto/serviceradar/pkg/core/auth 0.005s FAIL ``` # Full context The `RouteProtectionMiddleware` is applied to all protected API routes in the HTTP server. When a request comes in, it: 1. Extracts the authenticated user from the request context 2. Calls `getRequiredRoles` to determine which roles are needed for the requested path and HTTP method 3. If `getRequiredRoles` returns an empty array, the middleware interprets this as "no protection required" and allows the request to proceed without any role checks 4. Otherwise, it verifies that the user has at least one of the required roles The bug occurs in step 2-3: when an exact path match exists but doesn't define roles for the specific HTTP method being used, `getRequiredRoles` returns an empty array instead of falling back to wildcard pattern matches. This is particularly dangerous because: - The actual configuration files in this codebase (e.g., `packaging/core/config/core.json`, `helm/serviceradar/files/serviceradar-config.yaml`) use wildcard patterns like `/api/admin/*` to protect entire sections of the API - Developers might add method-specific rules for certain endpoints (like requiring `superadmin` for POST operations) without realizing this creates a security hole for other HTTP methods on that same endpoint - The middleware treats an empty roles array as "allow all" rather than "deny all" The bug is in `pkg/core/auth/rbac.go:72-90`, specifically the early return at line 79 that doesn't check if the returned roles array is empty before bypassing the wildcard pattern checks. # Why has this bug gone undetected? This bug has gone undetected for several reasons: 1. **No existing tests for the RBAC route protection logic**: The only test file in `pkg/core/auth/` is `auth_test.go`, which tests JWT authentication but not the route protection middleware or `getRequiredRoles` function. 2. **Limited production usage of method-specific rules**: Based on the configuration files in the codebase, most route protection uses simple role arrays rather than method-specific maps. The bug only manifests when you have BOTH a wildcard pattern AND an exact match with method-specific rules. 3. **Authentication middleware still active**: Even though the route protection is bypassed, the authentication middleware (`authenticationMiddleware`) still runs before the RBAC middleware (see `pkg/core/api/server.go:822-827`). This means completely unauthenticated requests are still blocked - only authenticated users without the required roles can exploit this bug. 4. **Common configurations don't trigger the bug**: Looking at the example configs, most routes either: - Use only wildcard patterns (works correctly) - Use only exact matches with method-specific rules (works, just doesn't inherit from wildcards) - Use exact matches with role arrays instead of method maps (works correctly) 5. **The bug requires specific configuration pattern**: You need someone to: - First define a wildcard protection (e.g., `/api/admin/*`: `["admin"]`) - Then add a more specific rule with method-specific roles (e.g., `/api/admin/users`: `{"POST": ["superadmin"]}`) - Then make a request with a different HTTP method (e.g., GET) This specific sequence is uncommon but not unrealistic - a developer might want to add stricter requirements for certain operations (POST/DELETE) while assuming the wildcard still protects read operations (GET). # Recommended fix The function should only return early from the exact match if it actually found roles for the requested method. If the exact match doesn't specify roles for the method, it should fall through to check wildcard patterns: ```go func getRequiredRoles(path, method string, routeProtection map[string]interface{}) []string { if routeProtection == nil { return []string{} } // Check for exact match first if protection, exists := routeProtection[path]; exists { roles := parseProtection(protection, method) if len(roles) > 0 { // <-- FIX 🟢 Only return if we found roles return roles } // Otherwise fall through to check wildcards } // Check for wildcard matches for pattern, protection := range routeProtection { if matchesPattern(path, pattern) { return parseProtection(protection, method) } } return []string{} } ``` This ensures that method-specific rules can override wildcard patterns when they specify roles for a method, but wildcard patterns still provide protection when the method-specific rule doesn't define roles for the requested HTTP method.

mfreeman451 commented

2026-03-28 04:27:25 +00:00

Author

Owner

Imported GitHub comment.

Original author: @mfreeman451
Original URL: https://github.com/carverauto/serviceradar/issues/2147#issuecomment-3663510876
Original created: 2025-12-17T03:45:03Z

closing as completed

Imported GitHub comment. Original author: @mfreeman451 Original URL: https://github.com/carverauto/serviceradar/issues/2147#issuecomment-3663510876 Original created: 2025-12-17T03:45:03Z --- closing as completed