Skip to content

Fix BOLA by enforcing per-user ownership for tasks and reports#484

Merged
utksh1 merged 6 commits into
utksh1:mainfrom
smirk-dev:fix/401-owner-authorization
Jun 4, 2026
Merged

Fix BOLA by enforcing per-user ownership for tasks and reports#484
utksh1 merged 6 commits into
utksh1:mainfrom
smirk-dev:fix/401-owner-authorization

Conversation

@smirk-dev
Copy link
Copy Markdown
Contributor

Summary

Closes #401.

Tasks, findings, and reports were globally addressable by ID with no
per-user ownership check — any authenticated caller could read, list,
delete, or export another user's scan results (Broken Object Level
Authorization / BOLA).

This PR introduces a per-user/per-workspace ownership model and scopes every
task/finding/report operation to the requesting owner.

Approach

SecuScan authenticates the deployment with a single shared API key. That gate
doesn't distinguish the users/workspaces sharing a deployment, which is what
enabled BOLA. We derive a stable owner identity per request from the
authenticated-user header (X-User-Id) — the same header the existing
resolve_client_identity already treats as the authenticated user — and fall
back to a shared DEFAULT_OWNER_ID for single-user deployments. In production
the header is expected to be set by an upstream auth proxy / SSO layer.

Key changes

Schema (database.py, migrations/003_add_owner_id.sql)

  • owner_id TEXT NOT NULL DEFAULT 'default' added to tasks, findings, reports.
  • Added idempotently for existing DBs via the PRAGMA-checked migration path
    (SQLite has no ADD COLUMN IF NOT EXISTS); the NOT NULL default backfills
    existing rows to the shared default owner.
  • Owner indexes on all three tables.

Auth (auth.py)

  • resolve_owner_id / get_current_owner dependency + DEFAULT_OWNER_ID.

Creation (executor.py, routes.py)

  • create_task accepts owner_id; POST /task/start persists the requester.
  • Findings and reports inherit their task's owner_id on every execution path.

Authorization (routes.py)

  • task/{id}/status|stream|result|cancel|report/{csv,html,pdf,sarif} → 403 on
    owner mismatch.
  • Single delete stays idempotent for missing tasks (200) but 403s on a
    foreign-owned task; bulk delete and clear operate only on the caller's tasks.
  • finding/{id} → 403 on mismatch.
  • tasks, findings, reports, dashboard/summary, attack-surface,
    assets filter by owner_id; cached lists are namespaced by owner so one
    user's data is never served to another.

Tests (test_owner_authorization.py)

  • 21 cross-user tests: User B cannot fetch/stream/cancel/delete/export User A's
    task, can't see A's tasks/findings/reports in any list/dashboard, and bulk
    delete/clear never touch A's data; User A retains full access to their own.

Migration / deployment notes

  • Backward compatible: existing rows and clients that don't send X-User-Id
    share owner_id = 'default' and keep their current behaviour.
  • The owner_id columns are added automatically on startup — no manual step.
  • Multi-tenant isolation requires an upstream layer to set X-User-Id.
  • Behaviour change: DELETE /tasks/clear now purges only the caller's history
    (the previous blind data-directory wipe was removed as it crossed owners).

Testing

  • python -m pytest testing/backend -m "not benchmark" → 937 passed, 3 skipped.
    (One unrelated Windows-only chmod permission test fails locally; passes on
    Linux CI.)
  • ruff check clean.

🤖 Generated with Claude Code

smirk-dev and others added 3 commits June 3, 2026 10:45
Introduce a per-user/per-workspace ownership column on the tasks, findings,
and reports tables to close the BOLA gap in issue utksh1#401, where any caller
could address another user's resources by ID.

- auth.resolve_owner_id / get_current_owner derive a stable owner identity
  from the authenticated-user header (X-User-Id), falling back to a shared
  DEFAULT_OWNER_ID for single-user deployments.
- owner_id columns are added idempotently (NOT NULL DEFAULT 'default') so
  existing rows are backfilled and single-user clients keep their history.
- Migration 003 adds owner indexes and a defensive backfill.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Populate owner_id at creation time and scope every read, list, delete, and
report/export endpoint to the requesting owner so cross-user access is
impossible (BOLA, issue utksh1#401).

- executor.create_task accepts owner_id; findings and reports inherit the
  owning task's owner_id on every execution path (manual, modular scanner,
  workflow, scheduler).
- task/report/finding GET/stream/cancel endpoints return 403 on owner
  mismatch; single delete stays idempotent for missing tasks but 403s on a
  foreign-owned task.
- list/aggregate endpoints (tasks, findings, reports, dashboard,
  attack-surface, assets) filter by owner_id and namespace their caches by
  owner so one user's data is never listed or served to another.
- bulk delete and clear operate only on the caller's own tasks.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Verify two distinct users (via X-User-Id) cannot reach each other's data:
fetch/stream/cancel/delete/report all return 403 across owners, list and
dashboard endpoints never leak another owner's tasks/findings/reports, and
bulk delete/clear only touch the caller's own tasks. Also confirm owners
retain full access to their own resources and missing-task delete stays
idempotent (issue utksh1#401).

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@smirk-dev
Copy link
Copy Markdown
Contributor Author

@madsysharma please check once, please add all relevant tags as well

@utksh1 utksh1 added level:critical 80 pts difficulty label for critical or high-impact PRs type:security Security work category bonus label type:testing Testing work category bonus label area:backend Backend API, database, or service work area:security Security-sensitive implementation or tests labels Jun 4, 2026
Copy link
Copy Markdown
Owner

@utksh1 utksh1 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewed the owner_id schema/migration, request owner resolution, endpoint scoping, cache namespacing, and cross-user integration tests. This closes the BOLA exposure while preserving default single-user behavior. Approved for merge once branch is current.

@utksh1 utksh1 added the gssoc:approved Admin validation: approved for GSSoC scoring label Jun 4, 2026
@utksh1 utksh1 merged commit 117abbe into utksh1:main Jun 4, 2026
7 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area:backend Backend API, database, or service work area:security Security-sensitive implementation or tests gssoc:approved Admin validation: approved for GSSoC scoring level:critical 80 pts difficulty label for critical or high-impact PRs type:security Security work category bonus label type:testing Testing work category bonus label

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Task and report APIs lack a user/owner authorization model

2 participants