Restrict deleting a fleet to global admins - #50271
Conversation
A fleet-scoped admin could delete their own assigned fleet via the API even though they cannot create one. DeleteTeam now requires the same global-scope write check as team creation.
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (1)
WalkthroughFleet deletion authorization now targets the generic team resource for write access instead of a team-specific resource. Tests validate global-write failures for technician and observer-plus roles, and the changelog documents the global-write requirement, including global administrators and GitOps. 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
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. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@changes/restrict-delete-fleet-to-global-admin`:
- Line 1: Update the changelog entry to state that fleet deletion requires
global write permissions, reflecting the authorization behavior and test
coverage that allow global GitOps as well as global admins; do not describe the
restriction as limited to global admins.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: 08599080-c139-4476-9f3d-ae05565bfeec
📒 Files selected for processing (3)
changes/restrict-delete-fleet-to-global-adminee/server/service/teams.goserver/service/teams_test.go
There was a problem hiding this comment.
Warning
- Copilot's review of this pull request may be incomplete because some of the changed files are excluded by your Copilot content exclusion settings. See Excluding content from Copilot for details.
Pull request overview
This PR aims to restrict deletion of a fleet/team so that only global admins can perform the operation, and updates authorization tests accordingly.
Changes:
- Updated EE
DeleteTeamauthorization to authorize writes at the global scope (via an emptyfleet.Teamobject). - Updated
TestTeamAuthto assertDeleteTeamuses “global write” authorization expectations. - Added a changes entry file (content excluded by policy).
Reviewed changes
Copilot reviewed 2 out of 3 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| server/service/teams_test.go | Adjusts the team auth test to use a global-write expectation for DeleteTeam. |
| ee/server/service/teams.go | Changes the DeleteTeam authorization check to authorize against an empty team object (global-scope write). |
| changes/restrict-delete-fleet-to-global-admin | Changes entry (content excluded by policy). |
Files excluded by content exclusion policy (1)
- changes/restrict-delete-fleet-to-global-admin
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| if err := svc.authz.Authorize(ctx, &fleet.Team{}, fleet.ActionWrite); err != nil { | ||
| return err |
| err = svc.DeleteTeam(ctx, 1) | ||
| checkAuthErr(t, tt.shouldFailTeamWrite, err) | ||
| checkAuthErr(t, tt.shouldFailGlobalWrite, err) |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #50271 +/- ##
=======================================
Coverage 68.16% 68.17%
=======================================
Files 3935 3935
Lines 250976 250985 +9
Branches 13443 13443
=======================================
+ Hits 171089 171098 +9
+ Misses 64554 64551 -3
- Partials 15333 15336 +3
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
The changes file said "global admins" but the fix (matching team creation) also allows global GitOps. Reworded to "global write permissions" and added explicit test coverage for technician and observer_plus (global and team-scoped), the two roles missing from the existing auth table, confirming they're blocked too.
There was a problem hiding this comment.
Warning
- Copilot's review of this pull request may be incomplete because some of the changed files are excluded by your Copilot content exclusion settings. See Excluding content from Copilot for details.
Pull request overview
Copilot reviewed 2 out of 3 changed files in this pull request and generated no new comments.
Files excluded by content exclusion policy (1)
- changes/restrict-delete-fleet-to-global-admin
Checklist for submitter
changes/,orbit/changes/oree/fleetd-chrome/changes.See Changes files for more information.
Testing
Summary by CodeRabbit