Skip to content

[DOM-75520] fix: enforce owner-based RBAC on all job endpoints#20

Open
ddl-subir-m wants to merge 1 commit intomainfrom
subir/pr7-rbac-owner
Open

[DOM-75520] fix: enforce owner-based RBAC on all job endpoints#20
ddl-subir-m wants to merge 1 commit intomainfrom
subir/pr7-rbac-owner

Conversation

@ddl-subir-m
Copy link
Copy Markdown
Collaborator

Why

Security issue: All job endpoints are currently accessible by any authenticated user regardless of ownership. A user can read, cancel, or delete jobs they don't own. The job listing endpoint also accepts a client-supplied owner parameter, which could be used to enumerate other users' jobs.

Additionally, the user identity resolution had the wrong priority: get_request_owner() preferred get_viewing_user() (sidecar API) over the domino-username header. In the App context, the sidecar always returns the App owner's identity, not the actual viewing user's. This meant all RBAC checks were comparing against the App owner instead of the browser user.

The clear_viewing_user() fix prevents user context from leaking between requests in the same process.

Depends on

Summary

Security fixes

  • _enforce_job_owner() on all single-job endpoints (get, status, metrics, progress, cancel, delete, logs) — returns 404 to non-owners to avoid leaking job existence
  • resolve_job_list_filters() ignores client-supplied owner — always resolves server-side from authenticated user
  • get_request_owner() inverted: prefers domino-username header (trusted, Domino proxy) over sidecar user (returns App owner)
  • clear_viewing_user() prevents cross-request user context leakage

Other improvements

  • JobListItemResponse lightweight schema for dashboard (excludes leaderboard/diagnostics blobs)
  • Owner filtering on cleanup operations and registered models
  • needs_request flag in compat route patterns for forwarding Request to RBAC endpoints
  • Background Domino sync throttling to avoid blocking list endpoints
  • Zombie local job detection for OOM'd training tasks

Files changed

  • app/services/job_service.py — RBAC enforcement, owner resolution, background sync
  • app/api/schemas/job.pyJobListItemResponse
  • app/api/routes/jobs.py — Request forwarding for owner enforcement
  • app/api/compat/patterns.pyneeds_request flag
  • app/api/compat/custom_jobs.py — owner enforcement on compat routes
  • app/api/compat/custom_models.py — owner filtering on models
  • app/services/model_service.py — owner param on registered models
  • app/core/context/user.pyclear_viewing_user()
  • app/core/leaderboard_utils.py — leaderboard normalization helpers (dependency)
  • app/main.py — clear user context after each request
  • tests/test_job_service.py — updated + new RBAC tests
  • tests/test_compat_jobs.py — compat route tests
  • tests/test_auth_header_concurrency.py — concurrency tests

Test plan

  • test_job_service.py passes
  • test_compat_jobs.py passes
  • test_auth_header_concurrency.py passes
  • Non-owner gets 404 on single-job endpoints
  • Job listing ignores client-supplied owner
  • domino-username header takes priority over sidecar

@ddl-subir-m ddl-subir-m requested review from a team and niole March 20, 2026 17:07
@ddl-subir-m ddl-subir-m changed the title fix: enforce owner-based RBAC on all job endpoints [DOM-75520] fix: enforce owner-based RBAC on all job endpoints Mar 23, 2026
@ddl-subir-m ddl-subir-m force-pushed the subir/pr7-rbac-owner branch from 61afe7f to f80ca44 Compare March 24, 2026 05:02
Build on PR #30's auth-actions foundation to cover more endpoints:

- JobListItemResponse lightweight schema for list views
- build_job_list_item_response() with best_model extraction
- summary_only CRUD optimization (skip large blobs in list queries)
- Owner scoping on cleanup/preview/orphan endpoints
- Owner filtering on registered models listing
- clear_viewing_user() to prevent cross-request context leakage
- Zombie local job detection for OOM'd training tasks
- Background sync throttling state (not yet wired)
- Python 3.9 compat: from __future__ import annotations in generated client

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
@ddl-subir-m ddl-subir-m force-pushed the subir/pr7-rbac-owner branch from bb15abf to 3513f67 Compare March 26, 2026 14:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant