3.3 KiB
Policy System Fix Summary
Issues Identified and Fixed
1. Test Compilation Issues
Problem: The comprehensive_test.go file had missing imports and couldn't compile.
Fix: Added the necessary imports (time, event, tag) and helper functions.
2. Critical Evaluation Order Bug
Problem: The policy evaluation order didn't match user expectations, particularly around the interaction between privileged events and allow lists.
Original Behavior:
- Privileged access always overrode allow lists
- Allow lists didn't properly grant access when users were found
Fixed Behavior:
- When BOTH
privileged: trueAND allow lists exist, allow lists are authoritative - Users in allow lists are properly granted access
- Privileged access only applies when no allow lists are specified
3. Missing Return Statements
Problem: When users were found in allow lists, the code didn't return true immediately but continued to other checks.
Fix: Added return true, nil statements after confirming user is in allow list.
Corrected Policy Evaluation Order
- Universal Constraints (size, tags, timestamps) - Apply to everyone
- Explicit Denials (deny lists) - Highest priority blacklist
- Privileged Access - Grants access ONLY if no allow lists exist
- Exclusive Allow Lists - When present, ONLY listed users get access
- Privileged Final Check - Deny non-involved users for privileged events
- Default Policy - Fallback when no rules apply
Key Behavioral Changes
Before Fix:
- Privileged users (author, p-tagged) could access events even if not in allow lists
- Allow lists were not properly returning true when users were found
- Test inconsistencies due to missing binary cache population
After Fix:
- Allow lists are authoritative when present (even over privileged access)
- Proper immediate return when user is found in allow list
- All tests pass including comprehensive requirements test
Test Results
All 5 requirements from Issue #5 are verified and passing:
- ✅ Requirement 1: Kind whitelist enforcement
- ✅ Scenario A: Write access control
- ✅ Scenario B: Read access control
- ✅ Scenario C: Privileged events (parties involved)
- ✅ Scenario D: Script-based validation
Important Configuration Notes
When configuring policies:
-
Allow lists are EXCLUSIVE: If you specify
write_alloworread_allow, ONLY those users can access. -
Privileged + Allow Lists: If you use both
privileged: trueAND allow lists, the allow list wins - even the author must be in the allow list. -
Privileged Only: If you use
privileged: truewithout allow lists, parties involved get automatic access. -
Deny Lists Trump All: Users in deny lists are always denied, regardless of other settings.
Files Modified
/pkg/policy/policy.go- Fixed evaluation order and added proper returns/pkg/policy/comprehensive_test.go- Fixed imports and compilation/docs/POLICY_TROUBLESHOOTING.md- Updated documentation with correct behavior/docs/POLICY_FIX_SUMMARY.md- This summary document
Verification
Run tests to verify all fixes:
# Run comprehensive requirements test
CGO_ENABLED=0 go test -v -run TestPolicyDefinitionOfDone ./pkg/policy
# Run all policy tests
CGO_ENABLED=0 go test ./pkg/policy