[API] Add rbac.restrict_all_users_mutations to reserve -u for admins - #10623
Open
SeungjinYang wants to merge 2 commits into
Open
[API] Add rbac.restrict_all_users_mutations to reserve -u for admins#10623SeungjinYang wants to merge 2 commits into
SeungjinYang wants to merge 2 commits into
Conversation
`--all-users`/`-u` on a mutating command fans the operation out over every
user's resources, so one user can tear down or cancel a teammate's work in a
single command. Add an opt-in API server config that reserves the flag for
admins on `sky down/stop/autostop/cancel` and `sky jobs cancel`. Read-only
uses of `-u` (`sky status`, `sky queue`, `sky jobs queue`) are unaffected.
Defaults to false, so an upgrade changes nothing. Admins are exempt, as is a
server with no per-user identity -- that exemption reuses
`role_filter.request_owner_scope`, which the rest of RBAC already treats as
"unrestricted caller".
Enforcement is split, by necessity:
- `sky cancel -u` / `sky jobs cancel -u` transmit `all_users` in the body,
so the API server rejects them with a 403. This holds for SDK callers.
- `sky down/stop/autostop -u` expand the flag into one request per cluster
in the client, so the server never sees it. For those three the check is
client-side, driven by a per-caller flag surfaced on `GET /api/health`.
Documented as a guardrail rather than a boundary; workspaces remain the
hard isolation mechanism.
Tested:
- `pytest tests/unit_tests/test_sky/server/test_role_filter.py
tests/unit_tests/test_sky/users/test_rbac.py
tests/unit_tests/test_sky/test_cli_restrict_all_users.py` (55 pass),
covering: restricted user denied, admin allowed, flag off by default,
no-auth server allowed, `-a/--all` never gated, and the CLI gate firing
for all five commands.
- FastAPI TestClient run against both endpoints confirming the body still
parses as JSON through the new `Depends` and that restricted -> 403,
admin -> 200, config-off -> 200.
- Manual: set `rbac.restrict_all_users_mutations: true` in
`~/.sky/config.yaml`, restart the API server (`sky api stop; sky api
start`), then as a non-admin confirm `sky down -u`, `sky stop -u`,
`sky autostop -u`, `sky cancel <cluster> -u` and `sky jobs cancel -u` are
all refused naming the config key, while `sky status -u`, `sky queue -u`,
`sky jobs queue -u` and every `-a/--all` variant still work; then repeat
as an admin and confirm all five are allowed.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
- config.rst referenced `:ref:` on a label that does not exist (`config-yaml-workspaces`), and the doc build treats warnings as errors. Use `<workspaces>`, the label the same file already links to at line 1723. - Adding a field to `ApiServerInfo` pushed its body from 9 lines to over pylint's `docstring-min-length=10`, so `missing-class-docstring` began firing on a class that had never needed a docstring. Give it one. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
rbac.restrict_all_users_mutations(defaultfalse), that reserves the--all-users/-uflag on mutating commands for admins:sky down -u,sky stop -u,sky autostop -u,sky cancel -u,sky jobs cancel -u. Without it, any user can tear down or cancel a teammate's work in a single command.-u(sky status,sky queue,sky jobs queue) are unaffected — this restricts what a user may do, not what they may see.role_filter.request_owner_scope, which the rest of RBAC already treats as "unrestricted caller".Enforcement is split, by necessity
sky cancel -u,sky jobs cancel -uall_usersis in the request body, soPOST /cancelandPOST /jobs/cancelreject with 403. Holds for SDK callers.sky down/stop/autostop -u-uinto one request per cluster in_down_or_stop_clusters, so the server never receives the flag and cannot reject it.For those three the check is therefore a guardrail, not a boundary: a caller driving the Python SDK directly is not stopped by it. The CLI drives it off a per-caller flag surfaced on
GET /api/health(already resolved server-side, so the client never has to reason about roles; an older server omits the field and the client defaults tofalse). This is called out in the docstring and in the config docs, which point at workspaces as the hard isolation mechanism.Making down/stop/autostop server-enforced needs a per-resource ownership check rather than a flag check — deliberately out of scope here, and a reasonable follow-up.
Implementation notes
sky/server/requests/role_filter.py— the two 403 gates areDepends()shims, matching the existingforce_caller_scope_cancel_body; the decision needs the caller's role, which only the dispatch context has.Test plan
Automated (55 pass):
covering: restricted user denied, admin allowed, flag off by default, no-auth server allowed,
-a/--allnever gated, and the CLI gate firing for all five commands.Also ran a FastAPI
TestClientagainst both endpoints to confirm the request body still parses as JSON through the newDepends, and that restricted → 403, admin → 200, config-off → 200.Manual:
rbac.restrict_all_users_mutations: truein~/.sky/config.yaml;sky api stop && sky api start.sky down -u,sky stop -u,sky autostop -u,sky cancel <cluster> -u,sky jobs cancel -uis refused with a message naming the config key.sky status -u,sky queue <cluster> -u,sky jobs queue -uand every-a/--allvariant still work.-uworks for everyone (default behavior unchanged).🤖 Generated with Claude Code