fix: enforce authorization on generic task kill/pause/unpause - #10271
fix: enforce authorization on generic task kill/pause/unpause#10271PerumalsamyR wants to merge 1 commit into
Conversation
|
Thank you for your pull request and welcome to our community. We could not parse the GitHub identity of the following contributors: PerumalsamyR.
|
8ad47c0 to
63dc1f4
Compare
|
Thank you for your pull request and welcome to our community. We require contributors to sign our Contributor License Agreement, and we don't seem to have the users @PerumalsamyR on file. In order for us to review and merge your code, please start the CLA process at https://determined.ai/cla. After we approve your CLA, we will update the contributors list (private) and comment |
63dc1f4 to
84a9f07
Compare
|
Thank you for your pull request and welcome to our community. We require contributors to sign our Contributor License Agreement, and we don't seem to have the users @PerumalsamyR on file. In order for us to review and merge your code, please start the CLA process at https://determined.ai/cla. After we approve your CLA, we will update the contributors list (private) and comment |
KillGenericTask, PauseGenericTask, and UnpauseGenericTask acted on a task looked up solely by ID, without any authorization check. Any authenticated user could cancel, pause, or unpause another user's generic tasks by supplying the task ID, allowing cross-user disruption of running workloads on a shared cluster. The sibling read endpoints GetTask and GetGenericTaskConfig already gate access through canDoActionsOnTask, which routes generic tasks through canAccessNTSCTask -> CanGetNSC. These three lifecycle endpoints skipped that step entirely. Add a canDoActionsOnTask check at the start of each handler so callers must be authorized for the task before it is killed, paused, or unpaused. Permission-denied is surfaced as NotFound, consistent with the existing endpoints, so task existence is not leaked. Add an integration test covering the denied-access path for all three endpoints.
84a9f07 to
fc230ba
Compare
|
Thank you for your pull request and welcome to our community. We require contributors to sign our Contributor License Agreement, and we don't seem to have the users @PerumalsamyR on file. In order for us to review and merge your code, please start the CLA process at https://determined.ai/cla. After we approve your CLA, we will update the contributors list (private) and comment |
1 participant
Summary
KillGenericTask,PauseGenericTask, andUnpauseGenericTaskinmaster/internal/api_generic_tasks.golooked up a task solely by ID and acted on it without any authorization check. As a result, any authenticated user could cancel, pause, or unpause another user's generic tasks just by supplying the task ID — a cross-user denial-of-service on shared multi-user clusters.This fixes #10270.
Root cause
The sibling read endpoints in the same subsystem already gate access:
GetTaskandGetGenericTaskConfig(master/internal/api_task.go) both calla.canDoActionsOnTask(...), which for generic/NTSC tasks routes throughcanAccessNTSCTask→CanGetNSC.KillCommand,KillNotebook,KillShell,KillTensorboard) all check the authz provider before acting.The three generic-task lifecycle endpoints skipped this step entirely.
Change
Add an
a.canDoActionsOnTask(ctx, taskID)check at the top of each of the three handlers, before the task is fetched and mutated — matching the pattern already used byGetGenericTaskConfig. Permission-denied is surfaced asNotFound, consistent with the existing endpoints, so task existence is not leaked to unauthorized callers.Testing
TestGenericTaskLifecycleAuthZ(master/internal/api_generic_tasks_intg_test.go), which mocks the NSC authz provider to deny access and asserts that all three endpoints reject the request. The test fails onmain(endpoints proceed without an authz call) and passes with this change.TestGetTask,TestGetGenericTaskConfig,TestAuthZCanTerminateNSC.Notes
No API/proto changes; the fix is limited to server-side authorization enforcement. Behavior for authorized callers is unchanged.