From 20ba1b73da331fee57d32baf20c2668128823f32 Mon Sep 17 00:00:00 2001 From: Naman Singh Date: Thu, 23 Jul 2026 13:16:48 +0530 Subject: [PATCH 1/3] fix(notifications): scope notification rules to owner_id to prevent cross-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 #2064 --- backend/secuscan/notification_service.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/backend/secuscan/notification_service.py b/backend/secuscan/notification_service.py index 223e18043..cd53034bf 100644 --- a/backend/secuscan/notification_service.py +++ b/backend/secuscan/notification_service.py @@ -653,8 +653,10 @@ async def process_finding_notifications( if not finding: return [] + owner_id = finding.get("owner_id", "default") rules = await db.fetchall( - "SELECT * FROM notification_rules WHERE is_active = 1 ORDER BY created_at ASC" + "SELECT * FROM notification_rules WHERE is_active = 1 AND owner_id = ? ORDER BY created_at ASC", + (owner_id,), ) results: List[DeliveryResult] = [] for rule in rules: From 91b2ed5953139dafba4a1d2fc87824a71f641966 Mon Sep 17 00:00:00 2001 From: Naman Singh Date: Fri, 24 Jul 2026 17:30:07 +0530 Subject: [PATCH 2/3] test(notifications): add two-owner regression tests for cross-tenant 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. --- .../integration/test_notification_routes.py | 115 ++++++++++++++++++ 1 file changed, 115 insertions(+) diff --git a/testing/backend/integration/test_notification_routes.py b/testing/backend/integration/test_notification_routes.py index 3d005cfc5..7e72ae22b 100644 --- a/testing/backend/integration/test_notification_routes.py +++ b/testing/backend/integration/test_notification_routes.py @@ -318,3 +318,118 @@ def test_admin_diagnostics_notifications(test_client, monkeypatch): assert "backoff_factor_seconds" in data assert type(data["max_retries"]) is int assert type(data["webhook_timeout_seconds"]) is float + + +# ── Cross-tenant notification isolation (regression tests) ──────────────────── + +OWNER_A = "user:alice" +OWNER_B = "user:bob" + + +def _seed_notification_rule_sync(owner_id: str, rule_id: str, name: str, + *, severity_threshold: str = "medium", + is_active: int = 1): + """Insert a notification rule directly with an explicit owner_id.""" + import sqlite3 + conn = sqlite3.connect(settings.database_path) + try: + conn.execute( + "INSERT INTO notification_rules " + "(id, name, owner_id, severity_threshold, channel_type, target_url_or_email, is_active) " + "VALUES (?, ?, ?, ?, 'webhook', 'https://example.com/hook', ?)", + (rule_id, name, owner_id, severity_threshold, is_active), + ) + conn.commit() + finally: + conn.close() + + +def _seed_finding_sync(owner_id: str, finding_id: str, task_id: str, + *, severity: str = "high"): + """Insert a finding directly with an explicit owner_id.""" + import sqlite3 + conn = sqlite3.connect(settings.database_path) + try: + conn.execute( + "INSERT INTO findings (id, owner_id, task_id, plugin_id, title, category, " + "severity, target, description, remediation) " + "VALUES (?, ?, ?, 'nmap', 'Open port', 'network', ?, '127.0.0.1', 'desc', 'fix')", + (finding_id, owner_id, task_id, severity), + ) + conn.commit() + finally: + conn.close() + + +def _seed_task_sync(owner_id: str, task_id: str): + """Insert a task directly with an explicit owner_id.""" + import sqlite3 + conn = sqlite3.connect(settings.database_path) + try: + conn.execute( + "INSERT INTO tasks (id, owner_id, plugin_id, tool_name, target, " + "status, inputs_json, structured_json, consent_granted) " + "VALUES (?, ?, 'nmap', 'nmap', '127.0.0.1', 'completed', '{}', " + "'{\"findings\": []}', 1)", + (task_id, owner_id), + ) + conn.commit() + finally: + conn.close() + + +@pytest.mark.asyncio +async def test_finding_only_triggers_own_owner_rules(test_client): + """A finding with owner_id A must only trigger rules owned by A, + never rules owned by B (cross-tenant notification isolation).""" + from backend.secuscan.database import get_db + from backend.secuscan.notification_service import process_finding_notifications + + db = await get_db() + + task_a = "task-owner-a" + task_b = "task-owner-b" + _seed_task_sync(OWNER_A, task_a) + _seed_task_sync(OWNER_B, task_b) + + _seed_notification_rule_sync(OWNER_A, "rule-a", "Alice Rule", + severity_threshold="medium") + _seed_notification_rule_sync(OWNER_B, "rule-b", "Bob Rule", + severity_threshold="medium") + + finding_a = "finding-owner-a" + _seed_finding_sync(OWNER_A, finding_a, task_a, severity="high") + + results = await process_finding_notifications(db, finding_a) + + triggered_rule_ids = [r.rule_id for r in results] + assert "rule-a" in triggered_rule_ids, ( + "Owner A's finding should trigger owner A's rule" + ) + assert "rule-b" not in triggered_rule_ids, ( + "Owner A's finding must NOT trigger owner B's rule (cross-tenant isolation)" + ) + + +@pytest.mark.asyncio +async def test_inactive_rules_not_triggered_for_any_owner(test_client): + """Inactive rules should not be triggered regardless of owner.""" + from backend.secuscan.database import get_db + from backend.secuscan.notification_service import process_finding_notifications + + db = await get_db() + + _seed_task_sync(OWNER_A, "task-inactive-test") + _seed_notification_rule_sync(OWNER_A, "rule-inactive", "Inactive Rule", + severity_threshold="medium", is_active=0) + _seed_notification_rule_sync(OWNER_A, "rule-active", "Active Rule", + severity_threshold="medium", is_active=1) + + _seed_finding_sync(OWNER_A, "finding-inactive-test", "task-inactive-test", + severity="high") + + results = await process_finding_notifications(db, "finding-inactive-test") + + triggered_rule_ids = [r.rule_id for r in results] + assert "rule-inactive" not in triggered_rule_ids + assert "rule-active" in triggered_rule_ids From 78c031e65bfad5bfc9a46ba2d10cfe166798323c Mon Sep 17 00:00:00 2001 From: Naman Singh Date: Fri, 24 Jul 2026 18:53:14 +0530 Subject: [PATCH 3/3] fix(tests): fix saved_views auth rejection tests using no_auth_app_client 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. --- testing/backend/unit/test_saved_views.py | 35 +++++++++++++++++++++--- 1 file changed, 31 insertions(+), 4 deletions(-) diff --git a/testing/backend/unit/test_saved_views.py b/testing/backend/unit/test_saved_views.py index 62c615051..09433fa9e 100644 --- a/testing/backend/unit/test_saved_views.py +++ b/testing/backend/unit/test_saved_views.py @@ -61,6 +61,33 @@ async def app_client(): _auth_module._api_key = None +@pytest_asyncio.fixture +async def no_auth_app_client(): + """A client with NO auth override, so require_api_key runs for real. + Used to test that unauthenticated / wrong-key requests are rejected.""" + test_db = Database(":memory:") + await test_db.connect() + _db_module.db = test_db + + _app = FastAPI() + _app.include_router(saved_views_router) + + with tempfile.TemporaryDirectory() as tmp_data_dir: + api_key = _auth_module.init_api_key(tmp_data_dir) + + transport = ASGITransport(app=_app) + async with AsyncClient( + transport=transport, + base_url="http://test", + ) as client: + client.api_key = api_key + yield client + + await test_db.disconnect() + _db_module.db = None + _auth_module._api_key = None + + @pytest_asyncio.fixture async def other_owner_client(app_client: AsyncClient): """A second authenticated client acting as a different owner (`bob`), @@ -355,18 +382,18 @@ async def test_filter_json_with_null_values_rejected(app_client: AsyncClient): # ─── Auth & owner isolation (issue #1743) ──────────────────────────────────── @pytest.mark.asyncio -async def test_unauthenticated_request_rejected(app_client: AsyncClient): +async def test_unauthenticated_request_rejected(no_auth_app_client: AsyncClient): """Requests without a valid API key/session are rejected, not served.""" - res = await app_client.get( + res = await no_auth_app_client.get( "/api/v1/saved-views", headers={"X-Api-Key": ""} ) assert res.status_code == 401 @pytest.mark.asyncio -async def test_wrong_api_key_rejected(app_client: AsyncClient): +async def test_wrong_api_key_rejected(no_auth_app_client: AsyncClient): """A malformed/incorrect API key is rejected.""" - res = await app_client.get( + res = await no_auth_app_client.get( "/api/v1/saved-views", headers={"X-Api-Key": "not-the-real-key"} ) assert res.status_code == 401