Skip to content

fix(notifications): scope rules to owner_id to prevent cross-tenant exfiltration (fixes #2064) - #2067

Open
namann5 wants to merge 5 commits into
utksh1:mainfrom
namann5:fix/cross-tenant-notification-exfiltration
Open

fix(notifications): scope rules to owner_id to prevent cross-tenant exfiltration (fixes #2064)#2067
namann5 wants to merge 5 commits into
utksh1:mainfrom
namann5:fix/cross-tenant-notification-exfiltration

Conversation

@namann5

@namann5 namann5 commented Jul 23, 2026

Copy link
Copy Markdown
Contributor

Summary

Fixes a high-severity cross-tenant notification data exfiltration vulnerability where process_finding_notifications queried ALL active notification rules without an owner_id filter.

Problem

The process_finding_notifications function in notification_service.py fetched all active notification rules across all tenants, causing every finding to trigger delivery to every rule owner's webhook endpoint. This allowed any authenticated user to passively exfiltrate other users' complete scan results by creating a notification rule with a low severity threshold.

Fix

  • Added owner_id filter to the notification rules query in process_finding_notifications
  • Rules are now scoped to the finding's owner, preventing cross-tenant data leakage

Impact

  • Prevents cross-tenant sensitive data exfiltration through notification channels
  • Maintains existing behavior for single-tenant deployments (owner = "default")

Testing

  • All existing tests pass
  • Ruff linting passes

Fixes #2064

…ross-tenant data exfiltration

The process_finding_notifications function queried ALL active notification
rules across ALL tenants without an owner_id filter. This allowed any
authenticated user to passively receive all other users' scan findings
by creating a notification rule pointing to their webhook.

Now the notification rules query filters by the finding's owner_id,
ensuring only rules belonging to the same tenant are evaluated.

Fixes utksh1#2064
@utksh1 utksh1 added level:critical 80 pts difficulty label for critical or high-impact PRs type:security Security work category bonus label type:bug Bug fix work category bonus label area:backend Backend API, database, or service work area:security Security-sensitive implementation or tests labels Jul 24, 2026

@utksh1 utksh1 left a comment

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

The owner-scoped query is the right fix, but this critical boundary needs regression coverage. Please add tests showing a finding only triggers its own owner’s active rules and never delivers to another owner’s rule, then rerun checks.

@utksh1 utksh1 left a comment

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

The query scoping is the right fix, but this security boundary needs direct regression coverage. Please add tests with two owners/rules proving a finding triggers only its owner rule and cannot deliver to another owner webhook, then rerun checks.

namann5 added 3 commits July 24, 2026 17:30
…rule isolation

Add tests proving a finding only triggers notification rules belonging
to its own owner and never delivers to another owner's webhook/rule.
Also adds a test verifying inactive rules are not triggered.
…ient

The app_client fixture overrides require_api_key to always succeed,
causing test_unauthenticated_request_rejected and
test_wrong_api_key_rejected to return 200 instead of 401.
Added no_auth_app_client fixture that uses real auth for these tests.
@namann5

namann5 commented Aug 4, 2026

Copy link
Copy Markdown
Contributor Author

@utksh1 fixed

@namann5
namann5 force-pushed the fix/cross-tenant-notification-exfiltration branch from 21bbd10 to 44bbc77 Compare August 5, 2026 15:44
@namann5

namann5 commented Aug 5, 2026

Copy link
Copy Markdown
Contributor Author

@utksh1 All admin review comments have been addressed and the branch is rebased onto the latest main (including the undici override fix), so all required checks are green and the PR is mergeable. Requesting re-review.

Summary of changes in this PR:

  • Cross-tenant notification isolation, with regression test est_finding_only_triggers_own_owner_rules covering that a finding only triggers rules for its own owner.
  • Rebased on current main; CI: backend lint/tests/unit, frontend checks, formatting-hygiene all pass.

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 level:critical 80 pts difficulty label for critical or high-impact PRs type:bug Bug fix work category bonus label type:security Security work category bonus label

Projects

None yet

Development

Successfully merging this pull request may close these issues.

CRITICAL: Cross-tenant notification data exfiltration via global rule query

2 participants