policy: add admin:PolicyName condition key - #275
harshavardhana wants to merge 2 commits into
Conversation
Policy admin actions (admin:CreatePolicy, admin:DeletePolicy,
admin:GetPolicy) take no resource, so a principal that may manage policies
may manage every policy, including its own, and can widen itself. The
admin:PolicyName condition key names the policy the action works on, so a
statement can limit those actions to a set of policies:
"Condition": {"StringLike": {"admin:PolicyName": ["app-*"]}}
The server fills PolicyName in for the policy admin handlers.
|
Navigate logical layers of code changes, visualize relationships, and explore their blast radius. Warning Review limit reachedNext included review available in 16 minutes. View limit detailsLimit details: You’ve used the included review currently available. You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. Review configuration: ⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Advanced Run ID: 📒 Files selected for processing (2)
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Advanced Run ID: 📒 Files selected for processing (2)
Included review availability: This review used your included allowance. Your plan provides up to 1 included review per hour; 0 remain after this review. 📝 WalkthroughWalkthroughThe change adds ChangesAdmin Policy Name Condition
Priority: ⬇️ Low Estimated code review effort: 2 (Simple) | ~10 minutes Change: Feature Merge Risk: ⚪ Minimal · up to No actionable issue remains in this change after normal checks. Security Architecture ReviewSecurity architecture risk: 🟡 Moderate · up to A name-based condition could restrict policy administration, but it protects requests only when the server supplies the actual target policy name. A name-specific Deny could be ineffective if used before that integration is in place. Retained concerns
Security review detailsSecurity Blast Radius
Security Findings and Attack Paths
Trust Boundaries and Controls
Hardening Proposals
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. A rabbit reads the policy key Comment |
admin:PolicyName describes an admin API request, so it must only validate on admin actions. It has to be in AllSupportedKeys to parse, which also put it in the s3:* key set; leave admin: keys out of that set.
Policy admin actions (
admin:CreatePolicy,admin:DeletePolicy,admin:GetPolicy) take no resource. A principal that may manage policies may therefore manage every policy, including its own, and can widen itself.The
admin:PolicyNamecondition key names the policy the action works on, so a statement can limit those actions to a set of policies:The server fills
PolicyNamein for the policy admin handlers (AIStor change follows once this merges). Orb uses it to let its apiserver manage only theorb-u-*andorb-systempolicies.Summary by CodeRabbit