fix(rbac): make wildcard-method rules reachable and resolve overlapping patterns deterministically (#3808)

docs/advanced-guide/rbac/page.md

@@ -206,6 +206,89 @@ For endpoints that need to match multiple paths, use mux patterns:
 - **Middle variable**: Use `"/api/{category}/posts"` instead of `"/api/*/posts"`
     - Matches: `/api/tech/posts`, `/api/news/posts`
 
+### Rule Resolution
+
+More than one endpoint entry can match the same request — a broad pattern and a narrow one, for
+example. GoFr resolves this by **most specific wins**, rather than by the order the entries appear
+in the config file.
+
+Specificity is compared segment by segment, and the first segment where two patterns differ decides:
+
+1. A literal segment (`users`) is more specific than
+2. a constrained variable (`{id:[0-9]+}`), which is more specific than
+3. a free variable (`{id}`), which is more specific than
+4. a multi-segment catch-all (`{path:.*}`).
+
+If the paths are equally specific, an entry that names its methods explicitly wins over one
+declared with `["*"]`.
+
+```json
+{
+  "endpoints": [
+    {
+      "path": "/admin/{path:.*}",
+      "methods": ["*"],
+      "requiredPermissions": ["admin:read"]
+    },
+    {
+      "path": "/admin/orgs/{org_id}",
+      "methods": ["DELETE"],
+      "requiredPermissions": ["admin:write"]
+    }
+  ]
+}
+```
+
+`DELETE /admin/orgs/123` matches both entries, and the second one governs it — so `admin:write` is
+required. `GET /admin/settings` matches only the first, so `admin:read` is required.
+
+`GET /admin/orgs/123` is the case to watch. The narrow entry is `["DELETE"]`-only, so it does not
+cover a GET at all and drops out on method before specificity is ever considered — the catch-all
+governs, and the request needs only `admin:read`. Writing a strict rule for one method does not
+protect the other methods on that path; each method needs its own entry, or the broad entry has to
+be strict enough to stand on its own.
+
+> **Note**: `"methods": ["*"]` — and omitting `methods` entirely, which means the same thing —
+> matches **every** HTTP method, including methods GoFr does not otherwise know about. Since an
+> entry states what a caller must have in order to be let through, covering an unrecognized method
+> tightens enforcement rather than relaxing it.
+
+#### Where the ordering is not decided
+
+**Two patterns that score identically** — `/{a}/{b}` and `/{x}/{y}` are the same shape, so nothing
+about the patterns separates them. One thing still does: an entry that **requires permissions wins
+over a public one**, because a tie is a config that did not express an intent either way and
+enforcing is the recoverable half of that mistake. Past that, the first entry declared wins. Avoid
+writing two entries that overlap without one being plainly narrower than the other.
+
+**A constraint that can span segments** — a constraint containing `/`, such as `{path:[a-z/]+}`,
+matches `/files/a/b/c` the way a catch-all does, so it is scored as one: least specific, losing to
+any narrower entry it overlaps. That is decided by the `/` appearing in the constraint at all, not
+by whether the regex can really produce one — `{id:[0-9]+/}` is scored as a catch-all even though
+it is anchored to a single segment. The effect is only ever to move an entry *down* the ordering,
+so it can lose to a narrower rule but never shadow one. Write multi-segment matches as `{path:.*}`
+or `{path:.+}` and the scoring is exact.
+
+#### Declaring the same path twice
+
+Two entries with the same method and path are one entry: **the last one declared wins, in full**.
+That includes the `public` flag — a public entry followed by a protected one for the same key is
+protected, not public. Duplicates are not rejected, so it is worth checking for them in a config
+assembled from more than one source.
+
+#### A pattern that cannot compile
+
+If a pattern's constraint is not valid regex — `{id:[}`, for example — the config still loads and
+the application still starts, but the pattern can never match a request, so **that endpoint is not
+enforced**. GoFr logs the pattern at error level on startup:
+
+```
+RBAC: endpoint[2]: invalid mux pattern: "/api/{id:[}": ... This endpoint will never match a
+request, so it is NOT enforced - any route it was meant to govern is currently unguarded.
+```
+
+Treat that line as an open route, not a warning about a typo.
+
 ## JWT-Based RBAC
 
 For production/public APIs, use JWT-based role extraction:
@@ -465,9 +548,18 @@ Or use role inheritance to avoid duplication:
 **Route not being protected by RBAC**
 - Verify the route is explicitly configured in `endpoints[]` array
 - Check that the path pattern matches exactly (case-sensitive)
-- Ensure HTTP method matches (or use `["*"]` for all methods)
+- Ensure HTTP method matches (or use `["*"]` for all methods) — a rule declared for one method does
+  not cover the others on that path
+- Check the startup logs for `invalid mux pattern` — a pattern whose constraint is not valid regex
+  loads but never matches, leaving that endpoint unenforced
 - Remember: Routes not in RBAC config are allowed to proceed (not blocked)
 
+**The wrong permission is being required on a path two entries match**
+- The more specific pattern governs — see [Rule Resolution](#rule-resolution)
+- Check whether the narrower entry actually covers the request's method; if it does not, it drops
+  out and the broader entry applies
+- If both patterns are equally specific, declaration order decides — rewrite one to be narrower
+
 ## How It Works
 
 1. **Role Extraction**: Extracts user role from header (`X-User-Role`) or JWT claims

pkg/gofr/rbac/config.go

@@ -125,6 +125,10 @@ type Config struct {
 	publicEndpointsMap    map[string]bool             `json:"-" yaml:"-"` // Key: "METHOD:/path", Value: true if public
 	endpointMap           map[string]*EndpointMapping `json:"-" yaml:"-"` // Key: "METHOD:/path", Value: endpoint object
 	muxRouter             *mux.Router                 `json:"-" yaml:"-"` // Used for mux pattern matching
+
+	// rules holds every (method, path) rule ordered most-specific-first, and is the
+	// single source of truth for resolving a request that no exact key matches.
+	rules []endpointRule `json:"-" yaml:"-"`
 }
 
 // LoadPermissions loads RBAC configuration from a JSON or YAML file.
@@ -221,6 +225,9 @@ func (c *Config) validateEndpointPath(path string, index int) error {
 		if err := validateMuxPattern(path); err != nil {
 			return fmt.Errorf("endpoint[%d]: invalid mux pattern: %w", index, err)
 		}
+
+		// Non-fatal: a pattern mux cannot compile loads, but never matches, so say so loudly.
+		c.logUncompilablePattern(path, index)
 	}
 
 	return nil
@@ -289,19 +296,24 @@ func (c *Config) buildRolePermissionsMap() {
 	}
 }
 
-// buildEndpointPermissionMap builds the endpoint permission map from Endpoints.
+// buildEndpointPermissionMap builds the endpoint permission map and the ordered rule list
+// from Endpoints.
 func (c *Config) buildEndpointPermissionMap() error {
-	for _, endpoint := range c.Endpoints {
+	for i := range c.Endpoints {
+		endpoint := &c.Endpoints[i]
+
 		methods := endpoint.Methods
 		if len(methods) == 0 {
 			methods = []string{"*"}
 		}
 
-		if err := c.processEndpointMethods(&endpoint, methods); err != nil {
+		if err := c.processEndpointMethods(endpoint, methods); err != nil {
 			return err
 		}
 	}
 
+	c.rules = buildEndpointRules(c.Endpoints)
+
 	return nil
 }
 
@@ -326,16 +338,24 @@ func buildEndpointKey(endpoint *EndpointMapping, methodUpper string) string {
 	return fmt.Sprintf("%s:%s", methodUpper, pattern)
 }
 
-// storeEndpointMapping stores an endpoint mapping.
+// storeEndpointMapping stores an endpoint mapping. A duplicate (method, path) declaration
+// overwrites the earlier one entirely - including the public flag and the permission list, which
+// live in separate maps. Leaving either behind would mix the two declarations together: a public
+// entry followed by a protected one for the same key would keep serving the route unauthenticated
+// while reporting the protected endpoint.
 func (c *Config) storeEndpointMapping(endpoint *EndpointMapping, key, methodUpper string) error {
 	// Store endpoint object for fast lookup
 	c.endpointMap[key] = endpoint
 
 	if endpoint.Public {
 		c.publicEndpointsMap[key] = true
+		delete(c.endpointPermissionMap, key)
+
 		return nil
 	}
 
+	delete(c.publicEndpointsMap, key)
+
 	if len(endpoint.RequiredPermissions) == 0 {
 		return fmt.Errorf("%w: %s %s", ErrEndpointMissingPermissions, methodUpper, endpoint.Path)
 	}
@@ -393,16 +413,15 @@ func (c *Config) GetRolePermissions(role string) []string {
 // Returns all required permissions (user needs ANY of them - OR logic).
 // Config is read-only after initialization, so no mutex is needed.
 func (c *Config) GetEndpointPermission(method, path string) ([]string, bool) {
-	methodUpper := strings.ToUpper(method)
-	key := fmt.Sprintf("%s:%s", methodUpper, path)
-
-	// Try exact match first
-	if perms, isPublic := c.checkExactMatch(key); isPublic || len(perms) > 0 {
-		return perms, isPublic
+	endpoint, isPublic := c.resolve(strings.ToUpper(method), path)
+	if endpoint == nil || isPublic {
+		return nil, isPublic
 	}
 
-	// Try pattern and regex matching
-	return c.checkPatternMatch(methodUpper, path)
+	permissions := make([]string, len(endpoint.RequiredPermissions))
+	copy(permissions, endpoint.RequiredPermissions)
+
+	return permissions, false
 }
 
 // getExactEndpoint returns the endpoint for an exact key match (O(1) lookup).
@@ -415,71 +434,3 @@ func (c *Config) getExactEndpoint(key string) (*EndpointMapping, bool) {
 
 	return nil, false
 }
-
-// checkExactMatch checks for an exact endpoint match.
-func (c *Config) checkExactMatch(key string) (permissions []string, isPublic bool) {
-	if public, ok := c.publicEndpointsMap[key]; ok && public {
-		return nil, true
-	}
-
-	if perms, ok := c.endpointPermissionMap[key]; ok {
-		return perms, false
-	}
-
-	return nil, false
-}
-
-// findEndpointByPattern finds an endpoint by pattern matching (wildcards/regex).
-// Only used when exact match fails, so this is O(n) but only for patterns.
-// Config is read-only after initialization, so no mutex is needed.
-func (c *Config) findEndpointByPattern(methodUpper, path string) (*EndpointMapping, bool) {
-	// Try pattern matching for wildcards/regex
-	// Iterate over endpointMap to find matching patterns
-	for key, endpoint := range c.endpointMap {
-		if c.matchesKey(key, methodUpper, path) {
-			isPublic := c.publicEndpointsMap[key]
-			return endpoint, isPublic
-		}
-	}
-
-	return nil, false
-}
-
-// checkPatternMatch checks for pattern and regex matches.
-// Config is read-only after initialization, so no mutex is needed.
-func (c *Config) checkPatternMatch(methodUpper, path string) (permissions []string, isPublic bool) {
-	// Try pattern matching for wildcards
-	for key, perms := range c.endpointPermissionMap {
-		if c.matchesKey(key, methodUpper, path) {
-			return perms, false
-		}
-	}
-
-	// Check public endpoints with pattern/regex
-	for key := range c.publicEndpointsMap {
-		if c.matchesKey(key, methodUpper, path) {
-			return nil, true
-		}
-	}
-
-	return nil, false
-}
-
-// matchesKey checks if a key matches the given method and path.
-// Keys are built by buildEndpointKey which uses Path (may contain mux patterns).
-// Uses mux Route.Match() for mux patterns, exact match for non-pattern paths.
-func (c *Config) matchesKey(key, methodUpper, path string) bool {
-	if !strings.HasPrefix(key, methodUpper+":") {
-		return false
-	}
-
-	pattern := strings.TrimPrefix(key, methodUpper+":")
-
-	// For exact paths (no variables), use string comparison
-	if !isMuxPattern(pattern) {
-		return pattern == path
-	}
-
-	// For mux patterns, use Route.Match() from endpoint_matcher
-	return matchMuxPattern(pattern, methodUpper, path, c.muxRouter)
-}