zjncs opened a new pull request, #1756:
URL: https://github.com/apache/casbin/pull/1756
## Description
`CachedEnforcer.RemovePolicies` and
`SyncedCachedEnforcer.checkManyAndRemoveCache` (used by both `AddPolicies` and
`RemovePolicies`) size the cache-key buffer from the **first** rule of the
batch and reuse it for every rule:
```go
irule := make([]interface{}, len(rules[0]))
for _, rule := range rules {
for i, param := range rule {
irule[i] = param
}
key, _ := e.getKey(irule...)
...
}
```
When the batch contains rules of different lengths (the `[][]string` API
does not require uniform length, and the model accepts variable-length rules —
e.g. an ABAC condition field makes rules longer than the plain RBAC ones):
- a later rule **longer** than the first panics with `index out of range`;
- a later rule **shorter** than the first keeps stale elements from the
previous rule at the tail, so the computed cache key is wrong (e.g.
`alice$$data1$$read$$extra` instead of `alice$$data1$$read$$`) and the **real
cached decision of a removed rule is never deleted**.
The second case is an authorization correctness bug, not just hygiene: after
`RemovePolicies` removes a rule from the model, `Enforce` keeps returning the
stale cached `true`:
```go
e, _ := NewCachedEnforcer("examples/rbac_model.conf",
"examples/rbac_policy.csv")
_, _ = e.AddPolicies([][]string{{"bob", "data2", "write", "extra"}})
e.Enforce("alice", "data1", "read") // true, cached
_, _ = e.RemovePolicies([][]string{
{"bob", "data2", "write", "extra"},
{"alice", "data1", "read"},
})
e.Enforce("alice", "data1", "read") // still true — rule was removed but the
cached decision survived
```
Fix: allocate the key buffer per rule (`make([]interface{}, len(rule))`
inside the loop) in both enforcers.
## Testing
Added 4 tests (`TestRemovePoliciesCacheRaggedRules{StaleEntry,LongerRule}`
in `enforcer_cached_test.go` and the `TestSync...` twins in
`enforcer_cached_synced_test.go`).
Before the fix (master `34297a1`):
- StaleEntry tests FAIL: `alice, data1, read: true, supposed to be false` —
stale cached decision
- LongerRule tests FAIL: `panic: runtime error: index out of range [3] with
length 3` in `checkManyAndRemoveCache`/`RemovePolicies`
After the fix: all 4 pass. All existing cache tests (`TestCache`,
`TestSyncCache`, cached g-function tests) still pass, and the full `go test
./...` suite passes (8 packages). `go vet` clean.
---
*This PR was prepared with AI assistance (GitHub Copilot / ZCode autonomous
agent). All findings were verified manually against the codebase before
submission.*
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
