From 1a2f05bafdf066d0bc65ba4a52f53052a1fc28f2 Mon Sep 17 00:00:00 2001 From: maldwg Date: Tue, 6 Oct 2026 11:59:27 +0200 Subject: [PATCH 1/7] fix schema issue for reports --- README.md | 2 + src/backend/main.py | 4 +- src/backend/models/parser_models.py | 4 +- src/backend/services/schema_migrations.py | 43 ++++++++++ .../crud/test_host_technology_locations.py | 84 +++++++++++++++++++ .../tests/services/test_schema_migrations.py | 61 ++++++++++++++ src/backend/tests/services/test_xml_parser.py | 21 +++++ src/database/vulnerability_scanner.sql | 2 +- 8 files changed, 218 insertions(+), 3 deletions(-) create mode 100644 src/backend/services/schema_migrations.py create mode 100644 src/backend/tests/crud/test_host_technology_locations.py create mode 100644 src/backend/tests/services/test_schema_migrations.py diff --git a/README.md b/README.md index 6069170..4b05499 100644 --- a/README.md +++ b/README.md @@ -40,6 +40,8 @@ It contains a `docker-compose.release.yml` pinned to the exact released image digests, an environment example, checksums, image metadata, and upgrade instructions. No source checkout or local image build is required. +Backend startup automatically widens `scan_host_technologies.location_name` to `LONGTEXT` on existing MariaDB installations, preserving complete host-technology location lists. The upgrade is idempotent, keeps existing values, and requires the application database user to have `ALTER` permission. Back up the database before upgrading; the first startup may take longer while this table is altered. Re-import reports that previously failed with a location-length error after deploying the updated backend. + The commands below start a development checkout: ```bash diff --git a/src/backend/main.py b/src/backend/main.py index cc31e6f..275cd18 100644 --- a/src/backend/main.py +++ b/src/backend/main.py @@ -172,9 +172,11 @@ async def startup_event(): from services.application_settings import apply_persisted_settings from services.greenbone_service import ensure_greenbone_credential_key from services.notification_jobs import ensure_notification_job_table - from core.db import SessionLocal + from services.schema_migrations import ensure_host_technology_location_capacity + from core.db import SessionLocal, engine ensure_notification_job_table() + ensure_host_technology_location_capacity(engine) db = SessionLocal() try: apply_persisted_settings(db) diff --git a/src/backend/models/parser_models.py b/src/backend/models/parser_models.py index 4d3aa13..7f4920f 100644 --- a/src/backend/models/parser_models.py +++ b/src/backend/models/parser_models.py @@ -11,6 +11,7 @@ Text, UniqueConstraint, ) +from sqlalchemy.dialects.mysql import LONGTEXT from sqlalchemy.orm import relationship from models.base import Base @@ -178,7 +179,8 @@ class ScanHostTechnology(Base): host_ip = Column(String(128), nullable=False, index=True) technology_type = Column(String(32), nullable=False) technology_name = Column(String(512), nullable=False) - location_name = Column(String(1024)) + # A technology can be observed at thousands of paths in one scan. + location_name = Column(Text().with_variant(LONGTEXT(), "mysql", "mariadb")) source_name = Column(String(255)) source_oid = Column(String(1024)) diff --git a/src/backend/services/schema_migrations.py b/src/backend/services/schema_migrations.py new file mode 100644 index 0000000..73439de --- /dev/null +++ b/src/backend/services/schema_migrations.py @@ -0,0 +1,43 @@ +"""Small, idempotent schema upgrades for existing installations.""" + +import logging + +from sqlalchemy import text +from sqlalchemy.engine import Engine + +logger = logging.getLogger(__name__) + + +def ensure_host_technology_location_capacity(engine: Engine) -> None: + """Preserve complete Greenbone location lists instead of limiting to 1024 chars.""" + if engine.dialect.name not in {"mysql", "mariadb"}: + return + + # Advisory locks belong to the connection, surviving MariaDB's DDL commits. + # All Uvicorn workers must check the column only after acquiring this lock. + lock_name = "vulnerabilityhub:host-technology-location" + with engine.begin() as connection: + acquired = connection.execute( + text("SELECT GET_LOCK(:name, 60)"), {"name": lock_name} + ).scalar() + if acquired != 1: + raise RuntimeError("Could not acquire host technology schema upgrade lock") + try: + column_type = connection.execute( + text( + "SELECT DATA_TYPE FROM information_schema.COLUMNS " + "WHERE TABLE_SCHEMA = DATABASE() " + "AND TABLE_NAME = 'scan_host_technologies' " + "AND COLUMN_NAME = 'location_name'" + ) + ).scalar() + if column_type in {"varchar", "tinytext", "text", "mediumtext"}: + connection.execute( + text( + "ALTER TABLE scan_host_technologies " + "MODIFY COLUMN location_name LONGTEXT NULL" + ) + ) + logger.info("Upgraded scan_host_technologies.location_name to LONGTEXT") + finally: + connection.execute(text("SELECT RELEASE_LOCK(:name)"), {"name": lock_name}) diff --git a/src/backend/tests/crud/test_host_technology_locations.py b/src/backend/tests/crud/test_host_technology_locations.py new file mode 100644 index 0000000..b2191ff --- /dev/null +++ b/src/backend/tests/crud/test_host_technology_locations.py @@ -0,0 +1,84 @@ +"""MariaDB regression coverage for long technology location lists and upgrades.""" + +from datetime import datetime + +import pytest +from sqlalchemy import text + +from crud.base import CRUDService +from models.parser_models import Host, Institution, Scan, ScanHostTechnology +from services.schema_migrations import ensure_host_technology_location_capacity + +pytestmark = pytest.mark.db + + +def test_upgrades_existing_location_column_and_preserves_long_values(db_session): + institution = Institution(name="Location regression") + host = Host(ip_address="192.0.2.10") + db_session.add_all([institution, host]) + db_session.flush() + scan = Scan( + report_id="long-locations", + institution_id=institution.id, + scan_date=datetime(2026, 10, 6), + file_path="/tmp/long-locations.xml", + ) + db_session.add(scan) + db_session.commit() + scan_id, host_id = scan.id, host.id + crud = CRUDService(db_session) + observation_id = crud.add_scan_host_technology( + scan_id, + host_id, + host.ip_address, + "application", + "cpe:/a:typo3:typo3", + location_name="/existing-path", + ) + db_session.commit() + engine = db_session.get_bind() + # Reproduce an existing deployment's schema, not just a newly created table. + with engine.begin() as connection: + connection.execute( + text( + "ALTER TABLE scan_host_technologies MODIFY COLUMN location_name VARCHAR(1024) NULL" + ) + ) + + ensure_host_technology_location_capacity(engine) + ensure_host_technology_location_capacity(engine) + db_session.expire_all() + assert ( + db_session.get(ScanHostTechnology, observation_id).location_name + == "/existing-path" + ) + + # Exceed TEXT's 64 KiB capacity as well as the original 1024-character limit. + locations = ", ".join( + f"/studium/lernkrankenhaus/seite-{index:04d}" for index in range(2500) + ) + assert len(locations) > 65535 + assert ( + crud.add_scan_host_technology( + scan_id, + host_id, + "192.0.2.10", + "application", + "cpe:/a:typo3:typo3", + location_name=locations, + ) + == observation_id + ) + new_id = crud.add_scan_host_technology( + scan_id, + host_id, + "192.0.2.10", + "application", + "cpe:/a:example:web", + location_name=locations, + commit=False, + ) + db_session.commit() + db_session.expire_all() + assert db_session.get(ScanHostTechnology, observation_id).location_name == locations + assert db_session.get(ScanHostTechnology, new_id).location_name == locations diff --git a/src/backend/tests/services/test_schema_migrations.py b/src/backend/tests/services/test_schema_migrations.py new file mode 100644 index 0000000..2e05915 --- /dev/null +++ b/src/backend/tests/services/test_schema_migrations.py @@ -0,0 +1,61 @@ +from unittest.mock import MagicMock + +import pytest +from sqlalchemy.exc import OperationalError + +from services.schema_migrations import ensure_host_technology_location_capacity + + +@pytest.mark.parametrize( + "column_type, expected_alters", + [("varchar", 1), ("text", 1), ("longtext", 0), (None, 0)], +) +def test_location_upgrade_is_idempotent(column_type, expected_alters): + engine = MagicMock() + engine.dialect.name = "mysql" + connection = engine.begin.return_value.__enter__.return_value + connection.execute.return_value.scalar.side_effect = [1, column_type] + + ensure_host_technology_location_capacity(engine) + + statements = [str(call.args[0]) for call in connection.execute.call_args_list] + assert sum(sql.startswith("ALTER TABLE") for sql in statements) == expected_alters + assert statements[0].startswith("SELECT GET_LOCK") + assert statements[-1].startswith("SELECT RELEASE_LOCK") + + +def test_location_upgrade_releases_lock_on_failure(): + engine = MagicMock() + engine.dialect.name = "mariadb" + connection = engine.begin.return_value.__enter__.return_value + locked = MagicMock() + locked.scalar.return_value = 1 + column = MagicMock() + column.scalar.return_value = "varchar" + connection.execute.side_effect = [ + locked, + column, + OperationalError("ALTER", {}, Exception("denied")), + MagicMock(), + ] + + with pytest.raises(OperationalError): + ensure_host_technology_location_capacity(engine) + assert str(connection.execute.call_args.args[0]).startswith("SELECT RELEASE_LOCK") + + +def test_location_upgrade_fails_if_lock_is_unavailable(): + engine = MagicMock() + engine.dialect.name = "mysql" + connection = engine.begin.return_value.__enter__.return_value + connection.execute.return_value.scalar.return_value = 0 + with pytest.raises(RuntimeError, match="schema upgrade lock"): + ensure_host_technology_location_capacity(engine) + assert connection.execute.call_count == 1 + + +def test_location_upgrade_skips_other_dialects(): + engine = MagicMock() + engine.dialect.name = "sqlite" + ensure_host_technology_location_capacity(engine) + engine.begin.assert_not_called() diff --git a/src/backend/tests/services/test_xml_parser.py b/src/backend/tests/services/test_xml_parser.py index da04a18..fe4c882 100644 --- a/src/backend/tests/services/test_xml_parser.py +++ b/src/backend/tests/services/test_xml_parser.py @@ -496,6 +496,27 @@ def test_extracts_report_host_technology_inventory(parser): assert application["source_oid"] == "app-source" +def test_preserves_long_host_technology_location_lists(parser): + root = etree.Element("report") + host = etree.SubElement(root, "host") + etree.SubElement(host, "ip").text = "192.0.2.10" + application = "cpe:/a:typo3:typo3" + detail = etree.SubElement(host, "detail") + etree.SubElement(detail, "name").text = "App" + etree.SubElement(detail, "value").text = application + locations = [f"/studium/lernkrankenhaus/seite-{index:04d}" for index in range(200)] + for location in locations: + detail = etree.SubElement(host, "detail") + etree.SubElement(detail, "name").text = application + etree.SubElement(detail, "value").text = location + parser.crud_service.add_host.return_value = 10 + + assert parser._process_host_technologies(root, scan_id=100) == 1 + stored = parser.crud_service.add_scan_host_technology.call_args.kwargs + assert stored["location_name"] == ", ".join(sorted(locations)) + assert len(stored["location_name"]) > 6000 + + def test_parse_file_integration(parser): with patch("os.path.isfile", return_value=True): with patch.object(parser, "_parse_xml_file") as mock_parse: diff --git a/src/database/vulnerability_scanner.sql b/src/database/vulnerability_scanner.sql index 27571bd..fdc122c 100644 --- a/src/database/vulnerability_scanner.sql +++ b/src/database/vulnerability_scanner.sql @@ -218,7 +218,7 @@ CREATE TABLE IF NOT EXISTS scan_host_technologies ( host_ip VARCHAR(128) NOT NULL, technology_type VARCHAR(32) NOT NULL, technology_name VARCHAR(512) NOT NULL, - location_name VARCHAR(1024), + location_name LONGTEXT, source_name VARCHAR(255), source_oid VARCHAR(1024), FOREIGN KEY (scan_id) REFERENCES scans(id) ON DELETE CASCADE, From 69a91e97ce808d853b180d148f7d5238059359f9 Mon Sep 17 00:00:00 2001 From: maldwg Date: Tue, 6 Oct 2026 12:02:30 +0200 Subject: [PATCH 2/7] fix bog that IPs were not deselcted after target deselction --- .github/workflows/ci.yml | 1 + src/frontend/app/e2e/smoke.spec.ts | 16 ++--- .../app/src/views/manage/ContactPersons.vue | 11 ++- .../manage/__tests__/ContactPersons.spec.ts | 70 ++++++++++++++++++- 4 files changed, 85 insertions(+), 13 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 5a33e74..f1c0a29 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -79,6 +79,7 @@ jobs: src/services/__tests__/csrf.spec.ts src/stores/__tests__/auth.spec.ts src/views/__tests__/Login.spec.ts + src/views/manage/__tests__/ContactPersons.spec.ts - name: Build frontend working-directory: ./src/frontend/app diff --git a/src/frontend/app/e2e/smoke.spec.ts b/src/frontend/app/e2e/smoke.spec.ts index 8555148..861f516 100644 --- a/src/frontend/app/e2e/smoke.spec.ts +++ b/src/frontend/app/e2e/smoke.spec.ts @@ -2540,6 +2540,9 @@ test.describe('dev admin smoke coverage', () => { const targetSelect = page.getByTestId('contact-person-greenbone-targets'); await expect(targetSelect).toContainText(greenboneTargetAName); await targetSelect.locator('.v-chip__close').click(); + await expect(page.getByTestId('contact-person-cidrs')).not.toContainText( + greenboneSharedCidr, + ); await page.getByTestId('contact-person-save-button').click(); await expect(page.getByTestId('contact-person-snackbar')).toContainText( 'Contact person updated successfully', @@ -2554,15 +2557,10 @@ test.describe('dev admin smoke coverage', () => { await expect(page.getByTestId('contact-person-snackbar')).toContainText( 'removed successfully', ); - page.once('dialog', async (dialog) => dialog.accept()); - await page - .getByTestId( - `contact-person-remove-cidr-${toTestId(followedContactAEmail)}-${toTestId(greenboneSharedCidr)}`, - ) - .click(); - await expect(page.getByTestId('contact-person-snackbar')).toContainText( - 'removed successfully', - ); + await expect(targetManagedCidrChip).toHaveCount(0); + await expect(contactPersonAssignmentChip( + page, followedContactBEmail, 'cidr', greenboneSharedCidr, + )).toBeVisible(); await page.goto('/manage/contact-persons'); await contactPersonRow(page, followedContactAEmail) diff --git a/src/frontend/app/src/views/manage/ContactPersons.vue b/src/frontend/app/src/views/manage/ContactPersons.vue index 2b04b47..eb06f45 100644 --- a/src/frontend/app/src/views/manage/ContactPersons.vue +++ b/src/frontend/app/src/views/manage/ContactPersons.vue @@ -284,7 +284,7 @@ closable-chips clearable :loading="targetsLoading" - hint="Current IPs and CIDRs from selected targets are added automatically and cannot be removed while the target is selected." + hint="Selected targets add and lock their IPs and CIDRs. Unfollowing automatically removes CIDRs unless another selected target uses them." persistent-hint @update:model-value="updateSelectedTargets" > @@ -844,6 +844,9 @@ export default { }, updateSelectedTargets(targetIds) { + const previouslyManagedCidrs = new Set( + this.editedTargetAssignments.flatMap(target => target.cidrs || []) + ); this.editedItem.greenbone_target_ids = targetIds || []; const candidates = new Map(); for (const target of (this.editedItem.greenbone_targets || [])) { @@ -864,10 +867,14 @@ export default { ]; this.editedItem.cidrs = [ ...new Set([ - ...(this.editedItem.cidrs || []), + ...(this.editedItem.cidrs || []).filter(cidr => !previouslyManagedCidrs.has(cidr)), ...selected.flatMap(target => target.cidrs || []), ]), ]; + this.editedItem.cidr_exclusions = Object.fromEntries( + Object.entries(this.editedItem.cidr_exclusions || {}) + .filter(([cidr]) => this.editedItem.cidrs.includes(cidr)) + ); }, updateEditedNetworks(kind, values) { diff --git a/src/frontend/app/src/views/manage/__tests__/ContactPersons.spec.ts b/src/frontend/app/src/views/manage/__tests__/ContactPersons.spec.ts index 7c42a78..5c15d83 100644 --- a/src/frontend/app/src/views/manage/__tests__/ContactPersons.spec.ts +++ b/src/frontend/app/src/views/manage/__tests__/ContactPersons.spec.ts @@ -6,16 +6,17 @@ import * as directives from 'vuetify/directives'; import ContactPersons from '../ContactPersons.vue'; -const { axiosGet, axiosPost } = vi.hoisted(() => ({ +const { axiosGet, axiosPost, axiosPut } = vi.hoisted(() => ({ axiosGet: vi.fn(), axiosPost: vi.fn(), + axiosPut: vi.fn(), })); vi.mock('axios', () => ({ default: { get: axiosGet, post: axiosPost, - put: vi.fn(), + put: axiosPut, delete: vi.fn(), }, })); @@ -51,7 +52,9 @@ describe('ContactPersons user autocompletes', () => { beforeEach(() => { axiosGet.mockReset(); axiosPost.mockReset(); + axiosPut.mockReset(); axiosPost.mockResolvedValue({ data: {} }); + axiosPut.mockResolvedValue({ data: {} }); axiosGet.mockImplementation((url: string) => { if (url === '/contact-persons/user-options') { return Promise.resolve({ @@ -87,6 +90,69 @@ describe('ContactPersons user autocompletes', () => { }); } + it('removes deselected target CIDRs and exclusions before creating a contact', async () => { + const wrapper = mountView(); + await flushPromises(); + const vm = wrapper.vm as any; + await vm.openCreateDialog(); + vm.availableGreenboneTargets = [{ + target_id: 'target-a', target_name: 'Target A', ips: [], cidrs: ['192.0.2.0/24'], + }]; + vm.editedItem.name = 'New owner'; + vm.editedItem.email = 'new@example.com'; + vm.updateEditedNetworks('cidrs', ['198.51.100.0/24']); + vm.updateSelectedTargets(['target-a']); + expect(vm.editedItem.cidrs).toEqual(['198.51.100.0/24', '192.0.2.0/24']); + vm.setCidrExclusions('192.0.2.0/24', ['192.0.2.9']); + vm.setCidrExclusions('198.51.100.0/24', ['198.51.100.9']); + + vm.updateSelectedTargets(null); // The selector's clear button. + expect(vm.editedItem.cidrs).toEqual(['198.51.100.0/24']); + expect(vm.editedItem.cidr_exclusions).toEqual({ + '198.51.100.0/24': ['198.51.100.9'], + }); + await vm.save(); + expect(axiosPost).toHaveBeenCalledWith('/contact-persons/', expect.objectContaining({ + cidrs: ['198.51.100.0/24'], + greenbone_target_ids: [], + cidr_exclusions: { '198.51.100.0/24': ['198.51.100.9'] }, + })); + wrapper.unmount(); + }); + + it('removes only unfollowed CIDRs when editing and keeps shared selected ranges', async () => { + const wrapper = mountView(); + await flushPromises(); + const vm = wrapper.vm as any; + const targets = [ + { target_id: 'target-a', target_name: 'A', ips: [], cidrs: ['192.0.2.0/24', '203.0.113.0/24'] }, + { target_id: 'target-b', target_name: 'B', ips: [], cidrs: ['203.0.113.0/24'] }, + ]; + const contact = { + id: 4, name: 'Existing owner', email: 'existing@example.com', ips: [], + cidrs: [ + { cidr_range: '192.0.2.0/24', excluded_ips: ['192.0.2.9'] }, + { cidr_range: '203.0.113.0/24', excluded_ips: ['203.0.113.9'] }, + { cidr_range: '198.51.100.0/24', excluded_ips: [] }, + ], + greenbone_target_ids: ['target-a', 'target-b'], greenbone_targets: targets, + }; + vm.contactPersons = [contact]; + // The saved snapshots must work even if target discovery is unavailable. + vm.availableGreenboneTargets = []; + vm.editItem(contact); + vm.updateSelectedTargets(['target-b']); + expect(vm.editedItem.cidrs).toEqual(['198.51.100.0/24', '203.0.113.0/24']); + expect(vm.editedItem.cidr_exclusions).toEqual({ + '198.51.100.0/24': [], '203.0.113.0/24': ['203.0.113.9'], + }); + await vm.save(); + expect(axiosPut).toHaveBeenCalledWith('/contact-persons/4', expect.objectContaining({ + cidrs: ['198.51.100.0/24', '203.0.113.0/24'], greenbone_target_ids: ['target-b'], + })); + wrapper.unmount(); + }); + it('integrates single-select user options into the name and email fields', async () => { const wrapper = mountView(); From 1bf989802a34da45acfe27d99a5ff346547bf614 Mon Sep 17 00:00:00 2001 From: maldwg Date: Tue, 6 Oct 2026 12:19:54 +0200 Subject: [PATCH 3/7] only send the most recent report if multiple are available nd mke that configurable --- .github/workflows/ci.yml | 1 + README.md | 6 + deploy/release/production.env.example | 1 + docs/getting_started/configuration.rst | 1 + example.env | 1 + src/backend/core/config.py | 1 + src/backend/crud/contact_persons.py | 10 +- src/backend/main.py | 6 +- src/backend/services/application_settings.py | 44 +++ src/backend/services/background.py | 13 + src/backend/services/greenbone_service.py | 20 +- src/backend/services/latest_reports.py | 81 ++++++ .../crud/test_contact_persons_extended.py | 4 + .../tests/services/test_background_helpers.py | 10 +- .../tests/services/test_latest_reports.py | 256 ++++++++++++++++++ src/database/vulnerability_scanner.sql | 1 + .../src/views/manage/ApplicationSettings.vue | 37 ++- .../__tests__/ApplicationSettings.spec.ts | 88 ++++++ 18 files changed, 565 insertions(+), 16 deletions(-) create mode 100644 src/backend/services/latest_reports.py create mode 100644 src/backend/tests/services/test_latest_reports.py create mode 100644 src/frontend/app/src/views/manage/__tests__/ApplicationSettings.spec.ts diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index f1c0a29..28e072f 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -80,6 +80,7 @@ jobs: src/stores/__tests__/auth.spec.ts src/views/__tests__/Login.spec.ts src/views/manage/__tests__/ContactPersons.spec.ts + src/views/manage/__tests__/ApplicationSettings.spec.ts - name: Build frontend working-directory: ./src/frontend/app diff --git a/README.md b/README.md index 4b05499..97944e5 100644 --- a/README.md +++ b/README.md @@ -79,6 +79,12 @@ reachable from containers at `ldap://ldap:389` and from the host at Upon first usage or database refresh, no user is available in the backend. Using the frontend you will be requested to setup an initial admin account which you can later on remove if need be. +### Report notification generations + +By default, synchronized Greenbone reports notify only for the newest imported report generation of each task. In **Application Settings > Email Automation**, turn **Only notify for the latest report per scan task** on or off and click **Save**; no restart is required. Turning it off allows older reports under the normal notification, access, and retention rules. Different task IDs remain independent even when they share a target. When enabled, older generations stay skipped even if they were never delivered or a contact was assigned later; historical resend requests follow the same rule. Per-host and institution exports belonging to the newest report remain eligible. Reports without a known Greenbone task ID are not grouped by filename, target, or IP, to avoid suppressing unrelated scans. + +Application Settings includes a search field matching names, descriptions, setting keys, and section names. Search for "latest report" to find this switch directly; filtering preserves unsaved changes. + ## Documentation 📚 **[Read the Full Documentation](https://vulnerabilityhub.readthedocs.io/en/latest/)** diff --git a/deploy/release/production.env.example b/deploy/release/production.env.example index 3008aff..8c7a104 100644 --- a/deploy/release/production.env.example +++ b/deploy/release/production.env.example @@ -24,6 +24,7 @@ SMTP_SENDER=VulnerabilityHub SMTP_USE_TLS=false SMTP_USE_STARTTLS=true SMTP_TIMEOUT_SECONDS=10 +REPORT_NOTIFICATIONS_LATEST_ONLY=true # Optional LDAP defaults LDAP_ENABLED=false diff --git a/docs/getting_started/configuration.rst b/docs/getting_started/configuration.rst index 9c89fa8..02942d6 100644 --- a/docs/getting_started/configuration.rst +++ b/docs/getting_started/configuration.rst @@ -74,6 +74,7 @@ These settings control when report notification emails are sent automatically. They can also be managed from **Manage > Configuration > Application Settings**. - ``AUTO_EMAIL_ENABLED``: Master switch for automated report emails (default: ``true``). +- ``REPORT_NOTIFICATIONS_LATEST_ONLY``: Notify only for the newest imported Greenbone report per task, including late contact assignments and historical resends (default: ``true``). Disable to allow older generations under normal delivery rules. The GUI override takes effect without a restart. - ``AUTO_EMAIL_ON_REPORT_MATCH``: Send report emails when uploaded XML and visual reports are matched (default: ``true``). - ``AUTO_EMAIL_ON_CONTACT_CREATE_ASSIGNMENT``: Send report emails when a newly created contact is assigned existing IPs or CIDRs (default: ``true``). - ``AUTO_EMAIL_ON_CONTACT_UPDATE_ASSIGNMENT``: Send report emails when an existing contact receives newly assigned IPs or CIDRs (default: ``true``). diff --git a/example.env b/example.env index 9f73562..34c190f 100644 --- a/example.env +++ b/example.env @@ -38,6 +38,7 @@ SMTP_SENDER=VulnerabilityHub SMTP_USE_TLS=false SMTP_USE_STARTTLS=false SMTP_TIMEOUT_SECONDS=10 +REPORT_NOTIFICATIONS_LATEST_ONLY=true # Development helpers MAILHOG_SMTP_PORT=1025 diff --git a/src/backend/core/config.py b/src/backend/core/config.py index ab04d93..c640861 100644 --- a/src/backend/core/config.py +++ b/src/backend/core/config.py @@ -183,6 +183,7 @@ class Settings(BaseSettings): # Automated report email triggers AUTO_EMAIL_ENABLED: bool = True + REPORT_NOTIFICATIONS_LATEST_ONLY: bool = True AUTO_EMAIL_ON_REPORT_MATCH: bool = True AUTO_EMAIL_ON_CONTACT_CREATE_ASSIGNMENT: bool = True AUTO_EMAIL_ON_CONTACT_UPDATE_ASSIGNMENT: bool = True diff --git a/src/backend/crud/contact_persons.py b/src/backend/crud/contact_persons.py index 4efe2dd..d5184e7 100644 --- a/src/backend/crud/contact_persons.py +++ b/src/backend/crud/contact_persons.py @@ -22,6 +22,7 @@ from models.user import User from schemas.contact_person import ContactPersonCreate, ContactPersonUpdate from services.application_settings import is_auto_email_enabled +from services.latest_reports import latest_notification_scans logger = logging.getLogger(__name__) @@ -1126,8 +1127,15 @@ def notify_contact_for_unnotified_scans( scans_map[scan.id] = {"scan": scan, "ips": set()} scans_map[scan.id]["ips"].add(ip) - # 2. Process each unique scan + latest_ids = { + scan.id for scan in latest_notification_scans( + db, [data["scan"] for data in scans_map.values()] + ) + } + # 2. Process only the latest generation of each independent task. for scan_id, data in scans_map.items(): + if scan_id not in latest_ids: + continue scan = data["scan"] relevant_ips = list(data["ips"]) diff --git a/src/backend/main.py b/src/backend/main.py index 275cd18..da0f958 100644 --- a/src/backend/main.py +++ b/src/backend/main.py @@ -169,7 +169,10 @@ async def target_managed_network_exception_handler( @app.on_event("startup") async def startup_event(): from crud.report_matches import repair_matched_visual_scan_dates - from services.application_settings import apply_persisted_settings + from services.application_settings import ( + apply_persisted_settings, + ensure_latest_report_setting, + ) from services.greenbone_service import ensure_greenbone_credential_key from services.notification_jobs import ensure_notification_job_table from services.schema_migrations import ensure_host_technology_location_capacity @@ -179,6 +182,7 @@ async def startup_event(): ensure_host_technology_location_capacity(engine) db = SessionLocal() try: + ensure_latest_report_setting(db) apply_persisted_settings(db) ensure_greenbone_credential_key(db) repaired_scan_dates = repair_matched_visual_scan_dates(db) diff --git a/src/backend/services/application_settings.py b/src/backend/services/application_settings.py index e7664cb..768ba92 100644 --- a/src/backend/services/application_settings.py +++ b/src/backend/services/application_settings.py @@ -3,6 +3,7 @@ from typing import Any from pydantic import ValidationError +from sqlalchemy.exc import IntegrityError from sqlalchemy.orm import Session from core.config import Settings, settings @@ -15,6 +16,49 @@ from models.application_setting import ApplicationSetting logger = logging.getLogger(__name__) +LATEST_REPORT_ONLY_KEY = "REPORT_NOTIFICATIONS_LATEST_ONLY" + + +def ensure_latest_report_setting(db: Session) -> None: + """Add the GUI toggle on existing installations without changing overrides.""" + if _get_definition(db, LATEST_REPORT_ONLY_KEY) is not None: + return + db.add( + ApplicationSetting( + key=LATEST_REPORT_ONLY_KEY, + label="Only notify for the latest report per scan task", + description=( + "Skip older Greenbone report generations, even if unsent or a contact " + "was assigned later. Independent task IDs remain separate. Disable to " + "allow older reports and historical resends under normal delivery rules." + ), + category="Email Automation", + value_type="boolean", + default_value="true", + read_only=False, + is_secret=False, + requires_restart=False, + ) + ) + try: + db.commit() + except IntegrityError: + # Concurrent startup workers may both try to insert the same key. + db.rollback() + if _get_definition(db, LATEST_REPORT_ONLY_KEY) is None: + raise + + +def latest_report_only_enabled(db: Session) -> bool: + """Read persisted policy on each send so workers do not use stale settings.""" + definition = _get_definition(db, LATEST_REPORT_ONLY_KEY) + if not isinstance(definition, ApplicationSetting): + return settings.REPORT_NOTIFICATIONS_LATEST_ONLY + return bool( + _deserialize(definition.value, definition) + if definition.value is not None + else _default_setting_value(definition) + ) def _jsonable(value: Any) -> Any: diff --git a/src/backend/services/background.py b/src/backend/services/background.py index 16db397..2a95ce9 100644 --- a/src/backend/services/background.py +++ b/src/backend/services/background.py @@ -40,6 +40,7 @@ ReportMatchService, ) from services.application_settings import is_auto_email_enabled +from services.latest_reports import latest_notification_scans logger = logging.getLogger(__name__) @@ -865,6 +866,15 @@ def send_report_notifications( if not scan: return + if not latest_notification_scans(db, [scan]): + return { + "message": "No email sent: a newer report exists for this scan task.", + "notifications_sent": 0, + "total_contacts": 0, + "already_notified": 0, + "errors": [], + } + scan_name = crud_scans.get_notification_scan_name(db, scan) # Ensure template has magic_link @@ -1159,6 +1169,9 @@ def send_notifications_for_unaccessed_reports( if not scans_with_results: return {"message": "No matching hosts found", "notifications_sent": 0} + # Select generations before checking access: an accessed new report + # must never make an older, unaccessed generation eligible again. + scans_with_results = latest_notification_scans(db, scans_with_results) # Filter to only scans that have never been accessed unaccessed_scans = [] for scan in scans_with_results: diff --git a/src/backend/services/greenbone_service.py b/src/backend/services/greenbone_service.py index a6c11c0..82051c5 100644 --- a/src/backend/services/greenbone_service.py +++ b/src/backend/services/greenbone_service.py @@ -2795,6 +2795,7 @@ def sync_reports(db: Session, run_id: int | None = None) -> GreenboneSyncRun: "available; visual attachments will be skipped.", visual.upper(), ) + pending_report_notifications = [] for report, monitored_task in report_entries: report_started = time.monotonic() report_id = report.report_id @@ -3185,11 +3186,8 @@ def process_artifact( _, visual_scan_id, ) in notification_visuals.items(): - _notify_greenbone_report_once( - db, - notification_xml_scan_id, - visual_scan_id, - auto_notify=config["auto_notify"], + pending_report_notifications.append( + (notification_xml_scan_id, visual_scan_id) ) finally: client.end_report() @@ -3204,6 +3202,18 @@ def process_artifact( time.monotonic() - report_started, _rss_megabytes(), ) + # Import every discovered generation before deciding which one to + # notify. Otherwise A1 could send before A2 has entered the database. + for ( + notification_xml_scan_id, + visual_scan_id, + ) in pending_report_notifications: + _notify_greenbone_report_once( + db, + notification_xml_scan_id, + visual_scan_id, + auto_notify=config["auto_notify"], + ) run.status, run.message = ( "completed", f"Imported {run.imported_count} artifact(s).", diff --git a/src/backend/services/latest_reports.py b/src/backend/services/latest_reports.py new file mode 100644 index 0000000..c845697 --- /dev/null +++ b/src/backend/services/latest_reports.py @@ -0,0 +1,81 @@ +"""Select the current report generation independently for each Greenbone task.""" + +from datetime import datetime, timezone + +from models.greenbone import GreenboneReportArtifact +from models.parser_models import Scan +from services.application_settings import latest_report_only_enabled + + +def latest_notification_scans(db, scans): + """Suppress older generations before delivery/access/recipient checks. + + Task identity, not target/IP/name, distinguishes independent scan series. + Reports without a known task ID remain independent rather than guessing + their series from a filename or the hosts they contain. + """ + if not scans: + return [] + if not latest_report_only_enabled(db): + return scans + artifacts = ( + db.query(GreenboneReportArtifact) + .filter( + GreenboneReportArtifact.local_scan_id.in_([scan.id for scan in scans]), + GreenboneReportArtifact.artifact_format == "xml", + GreenboneReportArtifact.status != "deleted", + ) + .all() + ) + tasks_by_scan = { + artifact.local_scan_id: artifact.task_id + for artifact in artifacts + if isinstance(artifact, GreenboneReportArtifact) and artifact.task_id + } + reports_by_scan = { + artifact.local_scan_id: artifact.report_id + for artifact in artifacts + if isinstance(artifact, GreenboneReportArtifact) and artifact.task_id + } + if not tasks_by_scan: + return scans + + # Include all newer generations, even if they were already delivered, lack + # a visual export, or no longer contain one of the contact's former hosts. + generations = ( + db.query( + Scan, GreenboneReportArtifact.task_id, GreenboneReportArtifact.report_id + ) + .join( + GreenboneReportArtifact, + GreenboneReportArtifact.local_scan_id == Scan.id, + ) + .filter( + GreenboneReportArtifact.task_id.in_(set(tasks_by_scan.values())), + GreenboneReportArtifact.artifact_format == "xml", + GreenboneReportArtifact.status != "deleted", + ) + .all() + ) + + def generation_key(scan): + date = scan.scan_date or datetime.min + if date.tzinfo is not None: + date = date.astimezone(timezone.utc).replace(tzinfo=None) + return date, scan.id + + newest = {} + for scan, task_id, report_id in generations: + if task_id not in newest or generation_key(scan) > generation_key( + newest[task_id][0] + ): + newest[task_id] = (scan, report_id) + return [ + scan + for scan in scans + if scan.id not in tasks_by_scan + or ( + tasks_by_scan[scan.id] in newest + and reports_by_scan[scan.id] == newest[tasks_by_scan[scan.id]][1] + ) + ] diff --git a/src/backend/tests/crud/test_contact_persons_extended.py b/src/backend/tests/crud/test_contact_persons_extended.py index cdd026d..4541cb6 100644 --- a/src/backend/tests/crud/test_contact_persons_extended.py +++ b/src/backend/tests/crud/test_contact_persons_extended.py @@ -11,6 +11,8 @@ from models.parser_models import Host, Institution, Scan, ScanResult, Vulnerability from models.report_file_match import ReportFileMatch from models.report_notification_delivery import ReportNotificationDelivery +from models.greenbone import GreenboneReportArtifact +from models.application_setting import ApplicationSetting @pytest.fixture @@ -209,6 +211,8 @@ def test_assignment_notifies_for_linked_greenbone_visual_only_once(self): ScanResult.__table__, ReportFileMatch.__table__, ReportNotificationDelivery.__table__, + GreenboneReportArtifact.__table__, + ApplicationSetting.__table__, ], ) db = sessionmaker(bind=engine)() diff --git a/src/backend/tests/services/test_background_helpers.py b/src/backend/tests/services/test_background_helpers.py index c953e1b..a1e84be 100644 --- a/src/backend/tests/services/test_background_helpers.py +++ b/src/backend/tests/services/test_background_helpers.py @@ -480,7 +480,7 @@ def test_send_report_notifications_returns_none_without_scan_id(self): def test_send_report_notifications_does_not_count_failed_templated_send(self): db = MagicMock() - scan = SimpleNamespace(file_path="/tmp/report.xml", file_hash="orig-hash") + scan = SimpleNamespace(id=5, file_path="/tmp/report.xml", file_hash="orig-hash") template = SimpleNamespace(subject="Subject", body="Body {magic_link}") contact = SimpleNamespace(name="Alice", email="alice@example.com", cidrs=[]) @@ -578,7 +578,7 @@ def test_send_report_notifications_identifies_report_by_scan_name_not_cidr(self) def test_send_report_notifications_notifies_all_contacts_for_same_ip(self): db = MagicMock() - scan = SimpleNamespace(file_path="/tmp/report.xml", file_hash="orig-hash") + scan = SimpleNamespace(id=5, file_path="/tmp/report.xml", file_hash="orig-hash") alice = SimpleNamespace(name="Alice", email="alice@example.com", cidrs=[]) bob = SimpleNamespace(name="Bob", email="bob@example.com", cidrs=[]) @@ -622,7 +622,7 @@ def test_send_report_notifications_notifies_all_contacts_for_same_ip(self): def test_send_report_notifications_limits_to_matched_host_ip(self): db = MagicMock() - scan = SimpleNamespace(file_path="/tmp/master.xml", file_hash="xml-hash") + scan = SimpleNamespace(id=5, file_path="/tmp/master.xml", file_hash="xml-hash") alice = SimpleNamespace(name="Alice", email="alice@example.com", cidrs=[]) def contacts_for_ip(db, ip_address): @@ -684,7 +684,7 @@ def test_send_report_notifications_suppresses_duplicate_delivery_email(self): xml_scan_id=5, recipient_email="alice@example.com" ) ) - scan = SimpleNamespace(file_path="/tmp/master.xml", file_hash="xml-hash") + scan = SimpleNamespace(id=5, file_path="/tmp/master.xml", file_hash="xml-hash") alice = SimpleNamespace(name="Alice", email="alice@example.com", cidrs=[]) with patch("services.background.crud_scans.get_scan", return_value=scan), patch( @@ -718,7 +718,7 @@ def test_send_report_notifications_suppresses_duplicate_delivery_email(self): def test_send_report_notifications_uses_all_xml_ips_for_master_visual(self): db = MagicMock() - scan = SimpleNamespace(file_path="/tmp/master.xml", file_hash="xml-hash") + scan = SimpleNamespace(id=5, file_path="/tmp/master.xml", file_hash="xml-hash") alice = SimpleNamespace(name="Alice", email="alice@example.com", cidrs=[]) bob = SimpleNamespace(name="Bob", email="bob@example.com", cidrs=[]) diff --git a/src/backend/tests/services/test_latest_reports.py b/src/backend/tests/services/test_latest_reports.py new file mode 100644 index 0000000..2ace610 --- /dev/null +++ b/src/backend/tests/services/test_latest_reports.py @@ -0,0 +1,256 @@ +from datetime import datetime +from types import SimpleNamespace +from unittest.mock import patch + +import pytest +from sqlalchemy import create_engine +from sqlalchemy.orm import sessionmaker + +from models.base import Base +from models.application_setting import ApplicationSetting +from models.greenbone import GreenboneReportArtifact +from models.parser_models import Institution, Scan, Host, Vulnerability, ScanResult +from models.report_file_match import ReportFileMatch +from models.report_notification_delivery import ReportNotificationDelivery +from models.report_access_log import ReportAccessLog +from services.latest_reports import latest_notification_scans +from crud.contact_persons import notify_contact_for_unnotified_scans +from services.application_settings import ( + ensure_latest_report_setting, + save_application_setting, + latest_report_only_enabled, +) +from core.config import settings + + +@pytest.fixture +def report_series(): + engine = create_engine("sqlite:///:memory:") + Base.metadata.create_all( + engine, + tables=[ + Institution.__table__, + ApplicationSetting.__table__, + Scan.__table__, + GreenboneReportArtifact.__table__, + Host.__table__, + Vulnerability.__table__, + ScanResult.__table__, + ReportFileMatch.__table__, + ReportNotificationDelivery.__table__, + ReportAccessLog.__table__, + ], + ) + db = sessionmaker(bind=engine)() + institution = Institution(name="Same target") + db.add(institution) + db.flush() + scans = [] + for name, task_id, day in [ + ("A2", "task-A", 2), + ("A1", "task-A", 1), + ("B1", "task-B", 3), + ]: + scan = Scan( + report_id=name, + institution_id=institution.id, + scan_date=datetime(2026, 10, day), + file_path=f"/tmp/{name}.xml", + ) + db.add(scan) + db.flush() + db.add( + GreenboneReportArtifact( + report_id=name, + task_id=task_id, + target_id="same-target", + artifact_format="xml", + scope="master", + local_scan_id=scan.id, + status="imported", + ) + ) + scans.append(scan) + db.commit() + try: + yield db, scans + finally: + db.close() + engine.dispose() + + +def test_latest_generation_is_selected_per_task_not_target(report_series): + db, (a2, a1, b1) = report_series + assert latest_notification_scans(db, [a1, a2, b1]) == [a2, b1] + + +def test_policy_toggle_applies_live_and_is_not_overwritten_on_upgrade(report_series): + db, (a2, a1, b1) = report_series + original = settings.REPORT_NOTIFICATIONS_LATEST_ONLY + try: + ensure_latest_report_setting(db) + row = ( + db.query(ApplicationSetting) + .filter_by(key="REPORT_NOTIFICATIONS_LATEST_ONLY") + .one() + ) + assert row.category == "Email Automation" + assert row.value_type == "boolean" + assert row.requires_restart is False + save_application_setting( + db, row.key, False, user_id=1, user_email="admin@example.com" + ) + # Simulate a worker whose in-memory settings have not been refreshed. + settings.REPORT_NOTIFICATIONS_LATEST_ONLY = True + assert latest_report_only_enabled(db) is False + assert latest_notification_scans(db, [a1, a2, b1]) == [a1, a2, b1] + ensure_latest_report_setting(db) + assert row.value == "false" + assert db.query(ApplicationSetting).filter_by(key=row.key).count() == 1 + save_application_setting( + db, row.key, True, user_id=1, user_email="admin@example.com" + ) + assert latest_notification_scans(db, [a1, a2, b1]) == [a2, b1] + finally: + settings.REPORT_NOTIFICATIONS_LATEST_ONLY = original + + +def test_older_unsent_report_does_not_reappear_when_newer_is_ineligible(report_series): + db, (a2, a1, b1) = report_series + # The caller may only consider A1 because A2 is accessed or delivered, + # lacks a visual export, or no longer contains a contact's old IP. + assert latest_notification_scans(db, [a1]) == [] + + +def test_unknown_task_reports_are_not_grouped_by_target(report_series): + db, (a2, a1, b1) = report_series + db.query(GreenboneReportArtifact).filter_by(local_scan_id=a1.id).delete() + db.commit() + assert latest_notification_scans(db, [a1, a2, b1]) == [a1, a2, b1] + + +def test_latest_generation_keeps_all_xml_slices_of_the_same_report(report_series): + db, (a2, a1, b1) = report_series + slice_scan = Scan( + report_id="A2-host", + institution_id=a2.institution_id, + scan_date=a2.scan_date, + file_path="/tmp/A2-host.xml", + ) + db.add(slice_scan) + db.flush() + db.add( + GreenboneReportArtifact( + report_id="A2", + task_id="task-A", + target_id="same-target", + artifact_format="xml", + scope="per_host", + host_ip="192.0.2.10", + local_scan_id=slice_scan.id, + status="completed", + ) + ) + db.commit() + assert latest_notification_scans(db, [a1, a2, slice_scan, b1]) == [ + a2, + slice_scan, + b1, + ] + + +def test_dispute_catchup_does_not_fall_back_to_unaccessed_old_generation(report_series): + from services.background import BackgroundService + + db, (a2, a1, b1) = report_series + host = Host(ip_address="192.0.2.10") + vulnerability = Vulnerability(nvt_id="dispute", name="Finding", severity=5) + db.add_all([host, vulnerability]) + db.flush() + for scan in [a1, a2]: + db.add( + ScanResult( + scan_id=scan.id, + host_id=host.id, + vulnerability_id=vulnerability.id, + host_ip=host.ip_address, + vulnerability_name="Finding", + vulnerability_nvt_id="dispute", + ) + ) + db.add(ReportAccessLog(scan_id=a2.id)) + db.commit() + with patch("services.background.send_templated_email") as send: + result = BackgroundService.send_notifications_for_unaccessed_reports( + db, + SimpleNamespace(email="late@example.com"), + ips=[host.ip_address], + ) + assert result["notifications_sent"] == 0 + send.assert_not_called() + + +def test_late_contact_receives_a2_and_b1_but_never_a1(report_series): + db, scans = report_series + host = Host(ip_address="192.0.2.10") + vulnerability = Vulnerability(nvt_id="test", name="Finding", severity=5) + db.add_all([host, vulnerability]) + db.flush() + for scan in scans: + db.add( + ScanResult( + scan_id=scan.id, + host_id=host.id, + vulnerability_id=vulnerability.id, + host_ip=host.ip_address, + vulnerability_name="Finding", + vulnerability_nvt_id="test", + ) + ) + db.commit() + contact = SimpleNamespace(name="Late contact", email="late@example.com") + with patch( + "crud.contact_persons.crud_scans.get_related_report_scans", + side_effect=lambda db, scan: [ + SimpleNamespace( + file_path=f"/tmp/{scan.report_id}.pdf", + file_hash=f"hash-{scan.report_id}", + id=scan.id, + ) + ], + ), patch( + "crud.contact_persons.crud_scans.get_notification_scan_name", + side_effect=lambda db, scan: scan.report_id, + ), patch( + "crud.contact_persons.send_templated_email", return_value=True + ) as send: + result = notify_contact_for_unnotified_scans(db, contact, [host.ip_address]) + assert {entry["report_id"] for entry in result["scans"]} == {"A2", "B1"} + assert send.call_count == 2 + assert {call.args[3]["scan_name"] for call in send.call_args_list} == { + "A2", + "B1", + } + # Previously delivered A2/B1 must not allow the still-unsent A1 back in. + assert ( + notify_contact_for_unnotified_scans(db, contact, [host.ip_address])[ + "notified" + ] + == 0 + ) + assert send.call_count == 2 + + +def test_direct_notification_refuses_superseded_report_even_when_forced(report_series): + from services.background import BackgroundService + + db, (a2, a1, b1) = report_series + with patch("services.background.crud_scans.get_scan", return_value=a1), patch( + "services.background.send_templated_email" + ) as send: + result = BackgroundService.send_report_notifications( + db, None, a1.id, force_notification=True + ) + assert result["notifications_sent"] == 0 + assert "newer report" in result["message"] + send.assert_not_called() diff --git a/src/database/vulnerability_scanner.sql b/src/database/vulnerability_scanner.sql index fdc122c..06363ad 100644 --- a/src/database/vulnerability_scanner.sql +++ b/src/database/vulnerability_scanner.sql @@ -112,6 +112,7 @@ VALUES ('SMTP_USE_STARTTLS', 'Use STARTTLS', 'Upgrade the SMTP connection with STARTTLS.', 'Email Delivery', 'boolean', 'false', FALSE, FALSE, FALSE, NULL, NULL), ('SMTP_TIMEOUT_SECONDS', 'SMTP timeout', 'Maximum seconds to wait for SMTP connect, TLS, auth, and send operations.', 'Email Delivery', 'float', '10.0', FALSE, FALSE, FALSE, 0.1, NULL), ('AUTO_EMAIL_ENABLED', 'Automated report emails', 'Master switch for automated report notification emails.', 'Email Automation', 'boolean', 'true', FALSE, FALSE, FALSE, NULL, NULL), +('REPORT_NOTIFICATIONS_LATEST_ONLY', 'Only notify for the latest report per scan task', 'Skip older Greenbone report generations, even if unsent or a contact was assigned later. Independent task IDs remain separate. Disable to allow older reports and historical resends under normal delivery rules.', 'Email Automation', 'boolean', 'true', FALSE, FALSE, FALSE, NULL, NULL), ('AUTO_EMAIL_ON_REPORT_MATCH', 'Report match notifications', 'Send report emails automatically when uploaded XML and visual report files are matched.', 'Email Automation', 'boolean', 'true', FALSE, FALSE, FALSE, NULL, NULL), ('AUTO_EMAIL_ON_CONTACT_CREATE_ASSIGNMENT', 'New contact assignment notifications', 'Send report emails when a newly created contact person is assigned existing IPs or CIDRs.', 'Email Automation', 'boolean', 'true', FALSE, FALSE, FALSE, NULL, NULL), ('AUTO_EMAIL_ON_CONTACT_UPDATE_ASSIGNMENT', 'Contact update assignment notifications', 'Send report emails when an existing contact person receives newly assigned IPs or CIDRs.', 'Email Automation', 'boolean', 'true', FALSE, FALSE, FALSE, NULL, NULL), diff --git a/src/frontend/app/src/views/manage/ApplicationSettings.vue b/src/frontend/app/src/views/manage/ApplicationSettings.vue index 3467902..fa3a497 100644 --- a/src/frontend/app/src/views/manage/ApplicationSettings.vue +++ b/src/frontend/app/src/views/manage/ApplicationSettings.vue @@ -56,13 +56,27 @@ + + - No application settings are available. + {{ searchQuery?.trim() ? 'No settings match your search. Try another term or clear the search.' : 'No application settings are available.' }} @@ -353,6 +367,7 @@ interface TestEmailResult { } const groups = ref([]); +const searchQuery = ref(''); const selectedCategory = ref(null); const loading = ref(false); const saving = ref(false); @@ -377,8 +392,22 @@ const notification = ref<{ message: '', }); +const filteredGroups = computed(() => { + const terms = (searchQuery.value || '').trim().toLowerCase().split(/\s+/).filter(Boolean); + if (!terms.length) return groups.value; + return groups.value.map(group => ({ + ...group, + settings: group.settings.filter(setting => { + // Do not search setting values, especially stored secrets. + const text = [group.category, setting.key, setting.label, setting.description] + .join(' ').toLowerCase(); + return terms.every(term => text.includes(term)); + }), + })).filter(group => group.settings.length > 0); +}); + const selectedGroup = computed(() => - groups.value.find(group => group.category === selectedCategory.value) || groups.value[0] || null + filteredGroups.value.find(group => group.category === selectedCategory.value) || filteredGroups.value[0] || null ); const hasRestartChanges = computed(() => diff --git a/src/frontend/app/src/views/manage/__tests__/ApplicationSettings.spec.ts b/src/frontend/app/src/views/manage/__tests__/ApplicationSettings.spec.ts new file mode 100644 index 0000000..ae8a70a --- /dev/null +++ b/src/frontend/app/src/views/manage/__tests__/ApplicationSettings.spec.ts @@ -0,0 +1,88 @@ +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'; +import { flushPromises, mount } from '@vue/test-utils'; +import { createVuetify } from 'vuetify'; +import * as components from 'vuetify/components'; +import * as directives from 'vuetify/directives'; +import ApplicationSettings from '../ApplicationSettings.vue'; + +const { get, patch } = vi.hoisted(() => ({ get: vi.fn(), patch: vi.fn() })); +vi.mock('axios', () => ({ default: { get, patch } })); +vi.mock('../../../stores/auth', () => ({ + useAuthStore: () => ({ user: { permissions: [] } }), +})); + +global.ResizeObserver = class { observe() {} unobserve() {} disconnect() {} }; +global.IntersectionObserver = class { observe() {} unobserve() {} disconnect() {} }; + +function setting(key: string, label: string, category: string, description: string, value: unknown) { + return { + key, label, category, description, value, default_value: value, + value_type: typeof value === 'boolean' ? 'boolean' : 'integer', + source: 'environment', overridden: false, is_secret: false, + has_value: true, read_only: false, requires_restart: false, + }; +} + +describe('Application settings search and policy toggle', () => { + const vuetify = createVuetify({ components, directives }); + let wrapper: ReturnType; + + beforeEach(async () => { + get.mockReset(); + patch.mockReset(); + get.mockResolvedValue({ data: { groups: [ + { category: 'Security', settings: [ + setting('REPORT_RETENTION_HOURS', 'Report link retention', 'Security', 'Hours before links expire', 24), + ] }, + { category: 'Email Automation', settings: [ + setting('REPORT_NOTIFICATIONS_LATEST_ONLY', 'Only notify for the latest report per scan task', 'Email Automation', 'Skip older Greenbone generations', true), + setting('AUTO_EMAIL_ENABLED', 'Automated report emails', 'Email Automation', 'Master switch', true), + ] }, + ] } }); + patch.mockResolvedValue({ data: {} }); + wrapper = mount(ApplicationSettings, { global: { + plugins: [vuetify], stubs: { 'v-dialog': { template: '
' } }, + } }); + await flushPromises(); + }); + + afterEach(() => wrapper.unmount()); + + async function search(value: string) { + await wrapper.get('[data-testid="application-settings-search"] input').setValue(value); + } + + it('finds settings across sections by label, description, key and section name', async () => { + expect(wrapper.find('[data-testid="application-setting-REPORT_RETENTION_HOURS"]').exists()).toBe(true); + for (const query of ['LATEST report', 'older Greenbone', 'REPORT_NOTIFICATIONS_LATEST_ONLY']) { + await search(query); + expect(wrapper.find('[data-testid="application-setting-REPORT_NOTIFICATIONS_LATEST_ONLY"]').exists()).toBe(true); + expect(wrapper.find('[data-testid="application-settings-category-security"]').exists()).toBe(false); + expect(wrapper.find('[data-testid="application-setting-AUTO_EMAIL_ENABLED"]').exists()).toBe(false); + } + await search('email automation'); + expect(wrapper.find('[data-testid="application-setting-AUTO_EMAIL_ENABLED"]').exists()).toBe(true); + await search(''); + expect(wrapper.find('[data-testid="application-settings-category-security"]').exists()).toBe(true); + }); + + it('shows no-match feedback and restores sections when cleared', async () => { + await search('no-such-setting'); + expect(wrapper.text()).toContain('No settings match your search'); + await search(''); + expect(wrapper.text()).not.toContain('No settings match your search'); + expect(wrapper.find('[data-testid="application-setting-REPORT_RETENTION_HOURS"]').exists()).toBe(true); + }); + + it('saves a changed toggle even after search hides it', async () => { + await search('latest'); + await wrapper.get('[data-testid="application-setting-input-REPORT_NOTIFICATIONS_LATEST_ONLY"] input').setValue(false); + await search('retention'); + expect(wrapper.find('[data-testid="application-setting-REPORT_NOTIFICATIONS_LATEST_ONLY"]').exists()).toBe(false); + await wrapper.get('[data-testid="application-settings-save-button"]').trigger('click'); + await flushPromises(); + expect(patch).toHaveBeenCalledWith('/admin/application-settings', { + updates: [{ key: 'REPORT_NOTIFICATIONS_LATEST_ONLY', value: false }], + }); + }); +}); From d4664142be697085f7582c9212b11e2b09da8f3c Mon Sep 17 00:00:00 2001 From: maldwg Date: Tue, 6 Oct 2026 12:24:10 +0200 Subject: [PATCH 4/7] dont prevent authenticated users to view reports when unauthentcated access is activated --- .github/workflows/ci.yml | 2 + src/frontend/app/e2e/smoke.spec.ts | 8 ++ src/frontend/app/src/__tests__/router.spec.ts | 41 +++++++++ src/frontend/app/src/router.ts | 6 +- .../app/src/views/public/ReportAccess.vue | 38 ++++---- .../public/__tests__/ReportAccess.spec.ts | 91 +++++++++++++++++++ 6 files changed, 169 insertions(+), 17 deletions(-) create mode 100644 src/frontend/app/src/__tests__/router.spec.ts create mode 100644 src/frontend/app/src/views/public/__tests__/ReportAccess.spec.ts diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 28e072f..23ee60e 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -81,6 +81,8 @@ jobs: src/views/__tests__/Login.spec.ts src/views/manage/__tests__/ContactPersons.spec.ts src/views/manage/__tests__/ApplicationSettings.spec.ts + src/views/public/__tests__/ReportAccess.spec.ts + src/__tests__/router.spec.ts - name: Build frontend working-directory: ./src/frontend/app diff --git a/src/frontend/app/e2e/smoke.spec.ts b/src/frontend/app/e2e/smoke.spec.ts index 861f516..76fe089 100644 --- a/src/frontend/app/e2e/smoke.spec.ts +++ b/src/frontend/app/e2e/smoke.spec.ts @@ -2933,6 +2933,14 @@ test.describe('dev admin smoke coverage', () => { const anonymousContext = await browser.newContext({ acceptDownloads: true }); const anonymousPage = await anonymousContext.newPage(); try { + // An existing account must use the same public magic-link flow. + await gotoReportAccess(page, magicLink); + await expect(page.getByText( + 'This magic link grants access without a user account.', + { exact: false }, + )).toBeVisible(); + await expect(page.getByTestId('report-access-not-responsible')).toHaveCount(0); + await gotoReportAccess(anonymousPage, magicLink); await expect( anonymousPage.getByText( diff --git a/src/frontend/app/src/__tests__/router.spec.ts b/src/frontend/app/src/__tests__/router.spec.ts new file mode 100644 index 0000000..cc0cc61 --- /dev/null +++ b/src/frontend/app/src/__tests__/router.spec.ts @@ -0,0 +1,41 @@ +import { beforeEach, describe, expect, it, vi } from 'vitest'; + +const { beforeEachGuard, initializeUser, checkAdminExists, auth } = vi.hoisted(() => ({ + beforeEachGuard: vi.fn(), + initializeUser: vi.fn(), + checkAdminExists: vi.fn(), + auth: { isLoggedIn: false, user: null as any }, +})); +vi.mock('vue-router', () => ({ + createWebHistory: vi.fn(), + createRouter: () => ({ beforeEach: beforeEachGuard }), +})); +vi.mock('../stores/auth', () => ({ useAuthStore: () => ({ ...auth, initializeUser }) })); +vi.mock('../services/adminBootstrap', () => ({ checkAdminExists })); +import '../router'; + +const guard = beforeEachGuard.mock.calls[0][0]; + +describe('report access routing', () => { + beforeEach(() => { + initializeUser.mockReset(); + checkAdminExists.mockReset(); + auth.isLoggedIn = false; + auth.user = null; + }); + + it.each([false, true])('leaves report access policy to the page (logged in: %s)', async (loggedIn) => { + auth.isLoggedIn = loggedIn; + auth.user = { mustChangePassword: true }; + initializeUser.mockRejectedValue(new Error('Expired session')); + expect(await guard({ path: '/reports/access/hash', meta: { reportAccess: true } })).toBe(true); + expect(initializeUser).not.toHaveBeenCalled(); + expect(checkAdminExists).not.toHaveBeenCalled(); + }); + + it('continues protecting other application pages', async () => { + checkAdminExists.mockResolvedValue(true); + expect(await guard({ path: '/profile', meta: { requiresAuth: true } })).toBe('/login'); + expect(initializeUser).toHaveBeenCalledOnce(); + }); +}); diff --git a/src/frontend/app/src/router.ts b/src/frontend/app/src/router.ts index 9f479e0..31ff36e 100644 --- a/src/frontend/app/src/router.ts +++ b/src/frontend/app/src/router.ts @@ -27,7 +27,7 @@ const routes = [ { path: '/register', component: () => import('./views/auth/Register.vue') }, { path: '/user-management', component: () => import('./views/manage/Users.vue'), meta: { requiresAuth: true, requiresPermission: 'users.manage' } }, { path: '/profile', component: () => import('./views/profile/Profile.vue'), meta: { requiresAuth: true } }, - { path: '/reports/access/:fileHash', component: () => import('./views/public/ReportAccess.vue') }, + { path: '/reports/access/:fileHash', component: () => import('./views/public/ReportAccess.vue'), meta: { reportAccess: true } }, { path: '/manage/disputes', component: () => import('./views/manage/Disputes.vue'), meta: { requiresAuth: true, requiresPermission: 'disputes.manage' } }, { path: '/manage/audit-logs', component: () => import('./views/manage/AuditLogs.vue'), meta: { requiresAuth: true, requiresPermission: 'audit.view' } }, ]; @@ -38,6 +38,10 @@ const router = createRouter({ }); router.beforeEach(async (to) => { + // The report page checks its access policy before consulting the session. + // Public magic links must not trigger token refresh or account redirects. + if (to.meta.reportAccess) return true; + const authStore = useAuthStore(); await authStore.initializeUser(); diff --git a/src/frontend/app/src/views/public/ReportAccess.vue b/src/frontend/app/src/views/public/ReportAccess.vue index 46c6563..924ee43 100644 --- a/src/frontend/app/src/views/public/ReportAccess.vue +++ b/src/frontend/app/src/views/public/ReportAccess.vue @@ -497,23 +497,25 @@ export default { }, async created() { - const authStore = useAuthStore(); - - if (!authStore.isLoggedIn) { - try { - const response = await axios.get('/public/report-access/config', { - withCredentials: true, - }); - this.csrfToken = response.headers['x-csrf-token'] || null; - this.anonymousAccess = Boolean( - response.data?.unauthenticated_access_enabled, - ); - } catch (error) { - console.error('Could not load report access configuration:', error); - this.anonymousAccess = false; - } + try { + const response = await axios.get('/public/report-access/config', { + withCredentials: true, + }); + this.csrfToken = response.headers['x-csrf-token'] || null; + this.anonymousAccess = Boolean( + response.data?.unauthenticated_access_enabled, + ); + } catch (error) { + console.error('Could not load report access configuration:', error); + this.error = 'Could not load report access configuration. Please try again.'; + this.loading = false; + return; + } - if (!this.anonymousAccess) { + if (!this.anonymousAccess) { + const authStore = useAuthStore(); + await authStore.initializeUser(); + if (!authStore.isLoggedIn) { this.$router.push({ path: '/login', query: { @@ -523,6 +525,10 @@ export default { }); return; } + if (authStore.user?.mustChangePassword) { + this.$router.push('/change-password-required'); + return; + } } await this.loadMetadata(); diff --git a/src/frontend/app/src/views/public/__tests__/ReportAccess.spec.ts b/src/frontend/app/src/views/public/__tests__/ReportAccess.spec.ts new file mode 100644 index 0000000..7d29300 --- /dev/null +++ b/src/frontend/app/src/views/public/__tests__/ReportAccess.spec.ts @@ -0,0 +1,91 @@ +import { beforeEach, describe, expect, it, vi } from 'vitest'; +import ReportAccess from '../ReportAccess.vue'; + +const { get, auth } = vi.hoisted(() => ({ + get: vi.fn(), + auth: { isLoggedIn: false, user: null as any, initializeUser: vi.fn() }, +})); +vi.mock('axios', () => ({ default: { get } })); +vi.mock('../../../stores/auth', () => ({ useAuthStore: () => auth })); + +function page() { + return { + ...ReportAccess.data(), + fileHash: 'a'.repeat(64), + $router: { push: vi.fn() }, + loadMetadata: vi.fn(), + }; +} + +describe('report link access policy', () => { + beforeEach(() => { + vi.resetAllMocks(); + auth.isLoggedIn = false; + auth.user = null; + get.mockResolvedValue({ + data: { unauthenticated_access_enabled: true }, + headers: { 'x-csrf-token': 'csrf' }, + }); + }); + + it.each(['logged out', 'unrelated account', 'password change required', 'expired session'])( + 'uses public report endpoints when enabled: %s', async (session) => { + auth.isLoggedIn = session !== 'logged out'; + auth.user = { permissions: [], mustChangePassword: session === 'password change required' }; + auth.initializeUser.mockRejectedValue(new Error('Session unavailable')); + const view = page(); + await ReportAccess.created.call(view); + expect(get).toHaveBeenCalledWith('/public/report-access/config', { withCredentials: true }); + expect(auth.initializeUser).not.toHaveBeenCalled(); + expect(view.$router.push).not.toHaveBeenCalled(); + expect(view.loadMetadata).toHaveBeenCalledOnce(); + for (const action of ['/metadata', '/acknowledge', '/download', '/download-host-reports']) { + expect(ReportAccess.methods.reportEndpoint.call(view, view.fileHash, action)) + .toBe(`/public/reports/${view.fileHash}${action}`); + } + expect(ReportAccess.methods.publicPostConfig.call(view)).toEqual({ + withCredentials: true, headers: { 'X-CSRF-Token': 'csrf' }, + }); + }, + ); + + it('requires login when public access is disabled', async () => { + get.mockResolvedValue({ data: { unauthenticated_access_enabled: false }, headers: {} }); + const view = page(); + await ReportAccess.created.call(view); + expect(auth.initializeUser).toHaveBeenCalledOnce(); + expect(view.$router.push).toHaveBeenCalledWith(expect.objectContaining({ path: '/login' })); + expect(view.loadMetadata).not.toHaveBeenCalled(); + }); + + it('uses account permissions when authentication is required', async () => { + get.mockResolvedValue({ data: { unauthenticated_access_enabled: false }, headers: {} }); + auth.isLoggedIn = true; + const view = page(); + await ReportAccess.created.call(view); + expect(auth.initializeUser).toHaveBeenCalledOnce(); + expect(view.loadMetadata).toHaveBeenCalledOnce(); + expect(ReportAccess.methods.reportEndpoint.call(view, view.fileHash, '/metadata')) + .toBe(`/reports/${view.fileHash}/metadata`); + }); + + it('keeps password-change requirements for authenticated access', async () => { + get.mockResolvedValue({ data: { unauthenticated_access_enabled: false }, headers: {} }); + auth.isLoggedIn = true; + auth.user = { mustChangePassword: true }; + const view = page(); + await ReportAccess.created.call(view); + expect(view.$router.push).toHaveBeenCalledWith('/change-password-required'); + expect(view.loadMetadata).not.toHaveBeenCalled(); + }); + + it('shows a configuration error without guessing the access policy', async () => { + get.mockRejectedValue(new Error('Offline')); + const view = page(); + await ReportAccess.created.call(view); + expect(view.loading).toBe(false); + expect(view.error).toContain('Could not load report access configuration'); + expect(view.$router.push).not.toHaveBeenCalled(); + expect(view.loadMetadata).not.toHaveBeenCalled(); + }); +}); From 5b799b217cb2f9c6c5a20f252d6cd3722a6dbc55 Mon Sep 17 00:00:00 2001 From: maldwg Date: Tue, 6 Oct 2026 12:58:08 +0200 Subject: [PATCH 5/7] fix bug where unauthenticated users can only fetch a single report --- src/backend/routes/reports.py | 37 ++++--- src/backend/tests/routes/test_reports.py | 119 ++++++++++++++++++++++- 2 files changed, 140 insertions(+), 16 deletions(-) diff --git a/src/backend/routes/reports.py b/src/backend/routes/reports.py index 0d2b834..e507abe 100644 --- a/src/backend/routes/reports.py +++ b/src/backend/routes/reports.py @@ -828,14 +828,17 @@ def _authorized_visual_bundle_files( ) -def _public_visual_bundle_files(db: Session, anchor_scan: Scan) -> List[Dict[str, Any]]: - """List variants covered by the exact public link without widening scope. +def _public_visual_bundle_files( + db: Session, anchor_scan: Scan, report_group: Optional[Dict[str, Any]] = None +) -> List[Dict[str, Any]]: + """List all visual variants in the public link's concrete scan execution. - A per-host visual link may reveal sibling formats for that same host, while - a master visual link may reveal only other master formats. An XML anchor is - similarly limited to master visuals. This avoids turning one host's link - into an index of every per-host artifact in the report family. + Public access grants the report delivery, not just the initially selected + host/format. Greenbone groups identify one report execution (not a task) + and also cover exports attached to separate per-host XML slices. """ + if report_group is None: + report_group = _report_group_for_scan(db, anchor_scan.id) anchor_path = (getattr(anchor_scan, "file_path", "") or "").lower() anchor_match = None xml_scan_id = anchor_scan.id if anchor_path.endswith(".xml") else None @@ -845,7 +848,14 @@ def _public_visual_bundle_files(db: Session, anchor_scan: Scan) -> List[Dict[str xml_scan_id = anchor_match.xml_scan_id candidates: List[Tuple[Scan, Optional[str]]] = [] - if xml_scan_id: + if report_group and report_group.get("source") == "greenbone": + candidates.extend( + (crud_scans.get_scan(db, item["scan_id"]), item.get("host_ip")) + for item in report_group.get("files", []) + if item.get("format") in {"PDF", "HTML"} + and item.get("status") == "completed" + ) + elif xml_scan_id: matches = ( db.query(ReportFileMatch) .filter( @@ -854,11 +864,10 @@ def _public_visual_bundle_files(db: Session, anchor_scan: Scan) -> List[Dict[str ) .all() ) - anchor_host_ip = getattr(anchor_match, "host_ip", None) candidates.extend( (match.visual_scan, match.host_ip) for match in matches - if match.visual_scan is not None and match.host_ip == anchor_host_ip + if match.visual_scan is not None ) else: candidates.extend( @@ -1171,7 +1180,7 @@ def get_public_report_metadata( report_name = ( report_group.get("title") if report_group else None ) or scan.report_id - files = _public_visual_bundle_files(db, scan) + files = _public_visual_bundle_files(db, scan, report_group) return { "report_id": scan.report_id, @@ -1414,7 +1423,7 @@ def download_public_single_host_reports( request: Request, db: Session = Depends(get_db), ): - """Download the same-host variants covered by an unauthenticated link.""" + """Download all single-host variants from the public link's delivery.""" if not download_request.confirm_responsibility: raise HTTPException(status_code=400, detail="Must confirm responsibility") _validate_public_magic_link_hash(file_hash) @@ -1422,8 +1431,11 @@ def download_public_single_host_reports( if not scan: raise HTTPException(status_code=404, detail="Report not found") + report_group = _report_group_for_scan(db, scan.id) host_files = [ - file for file in _public_visual_bundle_files(db, scan) if file.get("host_ip") + file + for file in _public_visual_bundle_files(db, scan, report_group) + if file.get("host_ip") ] host_scans = [ visual_scan @@ -1441,7 +1453,6 @@ def download_public_single_host_reports( detail="One or more report files are no longer available.", ) - report_group = _report_group_for_scan(db, scan.id) archive_path = _create_single_host_reports_archive(host_scans, report_group) try: _validate_report_download_retention(scan, db) diff --git a/src/backend/tests/routes/test_reports.py b/src/backend/tests/routes/test_reports.py index 23b4df2..6804992 100644 --- a/src/backend/tests/routes/test_reports.py +++ b/src/backend/tests/routes/test_reports.py @@ -1,10 +1,17 @@ import zipfile +from datetime import datetime from types import SimpleNamespace from unittest.mock import MagicMock, patch import pytest from fastapi import HTTPException +from sqlalchemy import create_engine +from sqlalchemy.orm import sessionmaker +from models.base import Base +from models.greenbone import GreenboneMonitoredTask, GreenboneReportArtifact +from models.parser_models import Institution, Scan +from models.report_file_match import ReportFileMatch from models.report_access_log import ReportAccessLog from models.dispute import Dispute, DisputeStatus, DisputeType from routes import reports @@ -49,7 +56,7 @@ def test_public_magic_link_rejects_malformed_hashes_before_database_lookup(): assert exc_info.value.status_code == 404 -def test_public_visual_bundle_does_not_expand_a_host_link_to_other_scopes(): +def test_public_visual_bundle_includes_all_matched_delivery_scopes(): db = MagicMock() anchor = SimpleNamespace(id=1, file_path="/tmp/host-a.pdf") anchor_match = SimpleNamespace( @@ -88,11 +95,117 @@ def test_public_visual_bundle_does_not_expand_a_host_link_to_other_scopes(): with patch( "routes.reports.crud_report_matches.get_match_for_scan", return_value=anchor_match, + ), patch( + "routes.reports._report_group_for_scan", return_value=None ): files = reports._public_visual_bundle_files(db, anchor) - assert [file["scan_id"] for file in files] == [2] - assert files[0]["host_ip"] == "192.0.2.10" + assert [file["scan_id"] for file in files] == [4, 2, 3] + assert files[0]["scope"] == "Complete report" + + +@pytest.mark.parametrize("anchor_kind", ["xml", "master", "per_host"]) +def test_public_delivery_lists_both_formats_and_scopes_for_one_execution(tmp_path, anchor_kind): + engine = create_engine("sqlite:///:memory:") + Base.metadata.create_all( + engine, + tables=[table for table in Base.metadata.sorted_tables if table.schema != "information_schema"], + ) + db = sessionmaker(bind=engine)() + try: + institution = Institution(name="Public report institute") + db.add_all([ + institution, + GreenboneMonitoredTask(task_id="task-a", task_name="Task A", visual_formats="xml:master,pdf:both,html:both"), + ]) + db.flush() + visuals = [] + anchors = {} + for scope, host_ip in [("master", ""), ("per_host", "192.0.2.10"), ("per_host", "192.0.2.11")]: + xml_scan = None + for extension in ["xml", "pdf", "html"]: + # Use a distinct persisted ID/hash for every exported artifact. + scan = Scan( + report_id=f"execution-a-{scope}-{host_ip}-{extension}", + institution_id=institution.id, + scan_date=datetime(2026, 10, 6), + file_path=str(tmp_path / f"{scope}-{host_ip or 'all'}.{extension}"), + file_hash=None, + ) + db.add(scan) + db.flush() + scan.file_hash = f"{scan.id:064x}" + (tmp_path / f"{scope}-{host_ip or 'all'}.{extension}").write_bytes(extension.encode()) + db.add(GreenboneReportArtifact( + report_id="execution-a", task_id="task-a", artifact_format=extension, + scope=scope, host_ip=host_ip, status="completed", local_scan_id=scan.id, + )) + if extension == "xml": + xml_scan = scan + if scope == "master": + anchors["xml"] = scan + else: + visuals.append(scan) + db.add(ReportFileMatch( + visual_scan_id=scan.id, xml_scan_id=xml_scan.id, + host_ip=host_ip or None, match_status="matched", + )) + anchors.setdefault(scope, scan) + + # Neither another run of this task nor another task on the same target + # belongs to the link's delivery. Deleted/failed artifacts stay hidden. + for report_id, task_id, status in [ + ("execution-a2", "task-a", "completed"), + ("execution-b", "task-b", "completed"), + ("execution-a", "task-a", "deleted"), + ("execution-a", "task-a", "failed"), + ]: + scan = Scan( + report_id=f"{report_id}-{status}", institution_id=institution.id, + scan_date=datetime(2026, 10, 6), file_path=f"/tmp/{report_id}-{status}.pdf", + ) + db.add(scan) + db.flush() + scan.file_hash = f"{scan.id:064x}" + db.add(GreenboneReportArtifact( + report_id=report_id, task_id=task_id, artifact_format="pdf", + scope="per_host", host_ip=f"192.0.2.{scan.id}", status=status, local_scan_id=scan.id, + )) + db.commit() + + anchor = anchors[anchor_kind] + metadata = reports.get_public_report_metadata(anchor.file_hash, db) + assert {item["scan_id"] for item in metadata["files"]} == {scan.id for scan in visuals} + assert {(item["format"], item["host_ip"]) for item in metadata["files"]} == { + (format_, host or None) + for format_ in ["PDF", "HTML"] + for host in ["", "192.0.2.10", "192.0.2.11"] + } + + request = SimpleNamespace(headers={}, client=SimpleNamespace(host="127.0.0.1")) + with patch("routes.reports._validate_report_download_retention"), patch("routes.reports.create_audit_log"): + for item in metadata["files"]: + download = reports.download_public_report( + item["file_hash"], reports.DownloadRequest(confirm_responsibility=True), + request=request, db=db, + ) + assert download.media_type == ("application/pdf" if item["format"] == "PDF" else "text/html") + assert download.path == str(tmp_path / item["filename"]) + + with patch("routes.reports._validate_report_download_retention"), patch("routes.reports.create_audit_log"): + response = reports.download_public_single_host_reports( + metadata["file_hash"], reports.DownloadRequest(confirm_responsibility=True), + request=request, db=db, + ) + try: + with zipfile.ZipFile(response.path) as archive: + assert len(archive.namelist()) == 4 + assert sorted(archive.read(name) for name in archive.namelist()) == [b"html", b"html", b"pdf", b"pdf"] + finally: + reports._remove_temporary_archive(response.path) + finally: + db.close() + engine.dispose() def test_send_report_notifications_queues_without_sending_in_request(): From c63e3d6660df03333dec95a32545839920c3e502 Mon Sep 17 00:00:00 2001 From: maldwg Date: Tue, 6 Oct 2026 16:24:35 +0200 Subject: [PATCH 6/7] correct wrong network information upon unauthenticated report access --- src/backend/routes/reports.py | 68 ++++++++++++++++++++++-- src/backend/tests/routes/test_reports.py | 45 +++++++++++++++- 2 files changed, 107 insertions(+), 6 deletions(-) diff --git a/src/backend/routes/reports.py b/src/backend/routes/reports.py index e507abe..37bb63e 100644 --- a/src/backend/routes/reports.py +++ b/src/backend/routes/reports.py @@ -285,6 +285,63 @@ def _require_admin(current_user: Any): raise HTTPException(status_code=403, detail="Not authorized") +def _get_public_access_network_context( + scan: Scan, db: Session, report_group: Optional[Dict[str, Any]] +) -> Tuple[List[int], List[str], List[str]]: + """Describe the full public delivery without relying on a user's mappings.""" + effective_scan, fallback_ips = _get_access_scan_and_ips(scan, db) + if report_group and report_group.get("source") == "greenbone": + group_files = [ + item for item in report_group.get("files", []) + if item.get("status") == "completed" + ] + scan_ids = {item["scan_id"] for item in group_files} + host_ips = {item["host_ip"] for item in group_files if item.get("host_ip")} + family_scans = db.query(Scan).filter(Scan.id.in_(scan_ids)).all() + else: + family_scans = crud_scans.get_related_report_scans(db, scan) + scan_ids = {item.id for item in family_scans} + host_ips = set() + + scan_ids.add(effective_scan.id) + host_ips.update(fallback_ips) + host_ips.update( + row[0] for row in db.query(ScanResult.host_ip) + .filter(ScanResult.scan_id.in_(scan_ids)).distinct().all() + if row[0] + ) + host_ips.update(item.target_ip for item in family_scans if item.target_ip) + parsed_ips = {} + for value in host_ips: + try: + parsed_ips[str(ipaddress.ip_address(value))] = ipaddress.ip_address(value) + except ValueError: + logger.warning("Skipping invalid report host IP: %s", value) + + institution_ids = {item.institution_id for item in family_scans} + matching_networks = {} + for (cidr,) in db.query(InstitutionCIDR.cidr_range).filter( + InstitutionCIDR.institution_id.in_(institution_ids) + ).all(): + try: + network = ipaddress.ip_network(cidr, strict=False) + except ValueError: + logger.warning("Skipping invalid institution CIDR: %s", cidr) + continue + if any(address in network for address in parsed_ips.values()): + matching_networks[str(network)] = network + + uncovered_ips = [ + value for value, address in parsed_ips.items() + if not any(address in network for network in matching_networks.values()) + ] + return ( + sorted(scan_ids), + sorted(parsed_ips), + [*sorted(matching_networks), *sorted(uncovered_ips)], + ) + + def _get_dispute_or_404(db: Session, dispute_id: int) -> Dispute: dispute = ( db.query(Dispute) @@ -1166,17 +1223,18 @@ def get_public_report_metadata( if not is_expired: retention_remaining = _report_retention_delta() - time_since_access - effective_scan, ip_list = _get_access_scan_and_ips(scan, db) - ip_list = sorted(set(ip_list)) + report_group = _report_group_for_scan(db, scan.id) + scan_ids, ip_list, network_scopes = _get_public_access_network_context( + scan, db, report_group + ) vuln_query = ( db.query(Vulnerability) .join(ScanResult) - .filter(ScanResult.scan_id == effective_scan.id) + .filter(ScanResult.scan_id.in_(scan_ids)) ) if ip_list: vuln_query = vuln_query.filter(ScanResult.host_ip.in_(ip_list)) vuln_count = vuln_query.count() - report_group = _report_group_for_scan(db, scan.id) report_name = ( report_group.get("title") if report_group else None ) or scan.report_id @@ -1190,7 +1248,7 @@ def get_public_report_metadata( "access_mode": "magic_link", "scan_date": scan.scan_date.isoformat() if scan.scan_date else None, "ip_addresses": ip_list, - "network_scopes": ip_list, + "network_scopes": network_scopes, "vulnerability_count": vuln_count, "last_accessed_at": ( effective_last_accessed_at.isoformat() diff --git a/src/backend/tests/routes/test_reports.py b/src/backend/tests/routes/test_reports.py index 6804992..8f12ad6 100644 --- a/src/backend/tests/routes/test_reports.py +++ b/src/backend/tests/routes/test_reports.py @@ -10,6 +10,7 @@ from models.base import Base from models.greenbone import GreenboneMonitoredTask, GreenboneReportArtifact +from models.institution_cidr import InstitutionCIDR from models.parser_models import Institution, Scan from models.report_file_match import ReportFileMatch from models.report_access_log import ReportAccessLog @@ -104,8 +105,35 @@ def test_public_visual_bundle_includes_all_matched_delivery_scopes(): assert files[0]["scope"] == "Complete report" +def test_public_network_context_uses_all_family_findings_and_mixed_ip_versions(): + db = MagicMock() + scan = SimpleNamespace(id=1) + xml = SimpleNamespace(id=2, institution_id=1, target_ip=None) + host_slice = SimpleNamespace(id=3, institution_id=1, target_ip="203.0.113.7") + findings = MagicMock() + findings.filter.return_value.distinct.return_value.all.return_value = [ + ("192.0.2.10",), ("192.0.2.11",), ("2001:db8::1",), + ("2001:db8::2",), ("invalid-host",), + ] + networks = MagicMock() + networks.filter.return_value.all.return_value = [ + ("192.0.2.5/24",), ("2001:db8::/64",), + ("198.51.100.0/24",), ("invalid-cidr",), + ] + db.query.side_effect = [findings, networks] + with patch("routes.reports._get_access_scan_and_ips", return_value=(xml, ["192.0.2.10"])), patch( + "routes.reports.crud_scans.get_related_report_scans", return_value=[xml, host_slice] + ): + scan_ids, ips, scopes = reports._get_public_access_network_context(scan, db, None) + + assert scan_ids == [2, 3] + assert ips == ["192.0.2.10", "192.0.2.11", "2001:db8::1", "2001:db8::2", "203.0.113.7"] + assert scopes == ["192.0.2.0/24", "2001:db8::/64", "203.0.113.7"] + + @pytest.mark.parametrize("anchor_kind", ["xml", "master", "per_host"]) -def test_public_delivery_lists_both_formats_and_scopes_for_one_execution(tmp_path, anchor_kind): +@pytest.mark.parametrize("network_mapping", ["cidr", "ips", "mixed"]) +def test_public_delivery_lists_both_formats_and_scopes_for_one_execution(tmp_path, anchor_kind, network_mapping): engine = create_engine("sqlite:///:memory:") Base.metadata.create_all( engine, @@ -119,6 +147,15 @@ def test_public_delivery_lists_both_formats_and_scopes_for_one_execution(tmp_pat GreenboneMonitoredTask(task_id="task-a", task_name="Task A", visual_formats="xml:master,pdf:both,html:both"), ]) db.flush() + cidrs = ["198.51.100.0/24"] # Not present in this report: never display it. + if network_mapping == "cidr": + cidrs.append("192.0.2.0/24") + elif network_mapping == "mixed": + cidrs.append("192.0.2.10/32") + db.add_all([ + InstitutionCIDR(institution_id=institution.id, cidr_range=cidr) + for cidr in cidrs + ]) visuals = [] anchors = {} for scope, host_ip in [("master", ""), ("per_host", "192.0.2.10"), ("per_host", "192.0.2.11")]: @@ -175,6 +212,12 @@ def test_public_delivery_lists_both_formats_and_scopes_for_one_execution(tmp_pat anchor = anchors[anchor_kind] metadata = reports.get_public_report_metadata(anchor.file_hash, db) + assert metadata["ip_addresses"] == ["192.0.2.10", "192.0.2.11"] + assert metadata["network_scopes"] == { + "cidr": ["192.0.2.0/24"], + "ips": ["192.0.2.10", "192.0.2.11"], + "mixed": ["192.0.2.10/32", "192.0.2.11"], + }[network_mapping] assert {item["scan_id"] for item in metadata["files"]} == {scan.id for scan in visuals} assert {(item["format"], item["host_ip"]) for item in metadata["files"]} == { (format_, host or None) From c16b35db141387657f5ed090fb5593733c12f97e Mon Sep 17 00:00:00 2001 From: maldwg Date: Wed, 7 Oct 2026 08:23:30 +0200 Subject: [PATCH 7/7] remove scehma migrations --- docs/development/development.rst | 49 ++++----------- src/backend/generate_demo_data.py | 1 - src/backend/main.py | 12 +--- src/backend/services/application_settings.py | 31 ---------- src/backend/services/notification_jobs.py | 14 +---- src/backend/services/schema_migrations.py | 43 ------------- .../tests/core/test_database_schema.py | 41 +++++++++++++ .../crud/test_host_technology_locations.py | 21 +++---- .../tests/services/test_latest_reports.py | 13 ++-- .../tests/services/test_schema_migrations.py | 61 ------------------- 10 files changed, 75 insertions(+), 211 deletions(-) delete mode 100644 src/backend/services/schema_migrations.py create mode 100644 src/backend/tests/core/test_database_schema.py delete mode 100644 src/backend/tests/services/test_schema_migrations.py diff --git a/docs/development/development.rst b/docs/development/development.rst index 225e077..9abc994 100644 --- a/docs/development/development.rst +++ b/docs/development/development.rst @@ -566,42 +566,19 @@ Frontend Debugging } -Database Migrations -------------------- - - -Manual Migration -~~~~~~~~~~~~~~~~ - - -1. Update ``database/vulnerability_scanner.sql`` -2. Create migration script: - -.. code-block:: sql - - -- migrations/001_add_my_feature.sql - ALTER TABLE my_table ADD COLUMN new_field VARCHAR(255); - - -3. Apply migration: - -.. code-block:: bash - - mariadb -u root -p vulnerabilityhub < migrations/001_add_my_feature.sql - - -Future: Alembic Integration -~~~~~~~~~~~~~~~~~~~~~~~~~~~ - - -Consider adding Alembic for automated migrations: - -.. code-block:: bash - - pip install alembic - alembic init alembic - alembic revision --autogenerate -m "Add my feature" - alembic upgrade head +Database Schema +--------------- + +``src/database/vulnerability_scanner.sql`` is the source of truth for the current +schema and built-in settings. Update this script together with the ORM models +when changing the database layout. The application and demo-data generator assume +an up-to-date database; they do not create tables, alter columns, or insert missing +setting definitions at runtime. + +Database initialization runs when provisioning a new database. Changing the SQL +script or restarting the application does not update an existing database volume. +Provision the required schema outside the application before starting it. Do not +delete a production database volume to apply a schema change. Performance Optimization diff --git a/src/backend/generate_demo_data.py b/src/backend/generate_demo_data.py index d65936a..a7f3989 100644 --- a/src/backend/generate_demo_data.py +++ b/src/backend/generate_demo_data.py @@ -463,7 +463,6 @@ def create_demo_data(): db = SessionLocal() try: print("Creating demo data...") - ReportFileMatch.__table__.create(db.get_bind(), checkfirst=True) create_demo_ldap_data(db) # 1. Create University Users and Contact Persons diff --git a/src/backend/main.py b/src/backend/main.py index da0f958..07bcc95 100644 --- a/src/backend/main.py +++ b/src/backend/main.py @@ -169,20 +169,12 @@ async def target_managed_network_exception_handler( @app.on_event("startup") async def startup_event(): from crud.report_matches import repair_matched_visual_scan_dates - from services.application_settings import ( - apply_persisted_settings, - ensure_latest_report_setting, - ) + from services.application_settings import apply_persisted_settings from services.greenbone_service import ensure_greenbone_credential_key - from services.notification_jobs import ensure_notification_job_table - from services.schema_migrations import ensure_host_technology_location_capacity - from core.db import SessionLocal, engine + from core.db import SessionLocal - ensure_notification_job_table() - ensure_host_technology_location_capacity(engine) db = SessionLocal() try: - ensure_latest_report_setting(db) apply_persisted_settings(db) ensure_greenbone_credential_key(db) repaired_scan_dates = repair_matched_visual_scan_dates(db) diff --git a/src/backend/services/application_settings.py b/src/backend/services/application_settings.py index 768ba92..0e21c6b 100644 --- a/src/backend/services/application_settings.py +++ b/src/backend/services/application_settings.py @@ -3,7 +3,6 @@ from typing import Any from pydantic import ValidationError -from sqlalchemy.exc import IntegrityError from sqlalchemy.orm import Session from core.config import Settings, settings @@ -19,36 +18,6 @@ LATEST_REPORT_ONLY_KEY = "REPORT_NOTIFICATIONS_LATEST_ONLY" -def ensure_latest_report_setting(db: Session) -> None: - """Add the GUI toggle on existing installations without changing overrides.""" - if _get_definition(db, LATEST_REPORT_ONLY_KEY) is not None: - return - db.add( - ApplicationSetting( - key=LATEST_REPORT_ONLY_KEY, - label="Only notify for the latest report per scan task", - description=( - "Skip older Greenbone report generations, even if unsent or a contact " - "was assigned later. Independent task IDs remain separate. Disable to " - "allow older reports and historical resends under normal delivery rules." - ), - category="Email Automation", - value_type="boolean", - default_value="true", - read_only=False, - is_secret=False, - requires_restart=False, - ) - ) - try: - db.commit() - except IntegrityError: - # Concurrent startup workers may both try to insert the same key. - db.rollback() - if _get_definition(db, LATEST_REPORT_ONLY_KEY) is None: - raise - - def latest_report_only_enabled(db: Session) -> bool: """Read persisted policy on each send so workers do not use stale settings.""" definition = _get_definition(db, LATEST_REPORT_ONLY_KEY) diff --git a/src/backend/services/notification_jobs.py b/src/backend/services/notification_jobs.py index 9980581..91f25b3 100644 --- a/src/backend/services/notification_jobs.py +++ b/src/backend/services/notification_jobs.py @@ -6,10 +6,9 @@ import uuid from datetime import datetime, timezone -from sqlalchemy.exc import OperationalError, ProgrammingError from sqlalchemy.orm import Session -from core.db import SessionLocal, engine +from core.db import SessionLocal from models.notification_job import NotificationJob logger = logging.getLogger(__name__) @@ -18,17 +17,6 @@ _notification_slots = threading.BoundedSemaphore(value=2) -def ensure_notification_job_table() -> None: - """Create the additive job table for both new and existing installations.""" - try: - NotificationJob.__table__.create(engine, checkfirst=True) - except (OperationalError, ProgrammingError) as exc: - # Multiple Uvicorn workers can race during startup. A duplicate-table - # result means another worker completed the same additive migration. - if "already exists" not in str(exc).lower(): - raise - - def serialize_notification_job(job: NotificationJob) -> dict: return { "job_id": job.job_id, diff --git a/src/backend/services/schema_migrations.py b/src/backend/services/schema_migrations.py deleted file mode 100644 index 73439de..0000000 --- a/src/backend/services/schema_migrations.py +++ /dev/null @@ -1,43 +0,0 @@ -"""Small, idempotent schema upgrades for existing installations.""" - -import logging - -from sqlalchemy import text -from sqlalchemy.engine import Engine - -logger = logging.getLogger(__name__) - - -def ensure_host_technology_location_capacity(engine: Engine) -> None: - """Preserve complete Greenbone location lists instead of limiting to 1024 chars.""" - if engine.dialect.name not in {"mysql", "mariadb"}: - return - - # Advisory locks belong to the connection, surviving MariaDB's DDL commits. - # All Uvicorn workers must check the column only after acquiring this lock. - lock_name = "vulnerabilityhub:host-technology-location" - with engine.begin() as connection: - acquired = connection.execute( - text("SELECT GET_LOCK(:name, 60)"), {"name": lock_name} - ).scalar() - if acquired != 1: - raise RuntimeError("Could not acquire host technology schema upgrade lock") - try: - column_type = connection.execute( - text( - "SELECT DATA_TYPE FROM information_schema.COLUMNS " - "WHERE TABLE_SCHEMA = DATABASE() " - "AND TABLE_NAME = 'scan_host_technologies' " - "AND COLUMN_NAME = 'location_name'" - ) - ).scalar() - if column_type in {"varchar", "tinytext", "text", "mediumtext"}: - connection.execute( - text( - "ALTER TABLE scan_host_technologies " - "MODIFY COLUMN location_name LONGTEXT NULL" - ) - ) - logger.info("Upgraded scan_host_technologies.location_name to LONGTEXT") - finally: - connection.execute(text("SELECT RELEASE_LOCK(:name)"), {"name": lock_name}) diff --git a/src/backend/tests/core/test_database_schema.py b/src/backend/tests/core/test_database_schema.py new file mode 100644 index 0000000..acef0c5 --- /dev/null +++ b/src/backend/tests/core/test_database_schema.py @@ -0,0 +1,41 @@ +"""The initialization SQL owns the schema; application code must not mutate it.""" + +import ast +from pathlib import Path + +import pytest + +from models.application_setting import ApplicationSetting + + +def test_runtime_code_does_not_create_or_alter_database_schema(): + backend_root = Path(__file__).resolve().parents[2] + violations = [] + for path in backend_root.rglob("*.py"): + if "tests" in path.relative_to(backend_root).parts: + continue + for node in ast.walk(ast.parse(path.read_text(), filename=str(path))): + if isinstance(node, ast.Constant) and isinstance(node.value, str): + sql = " ".join(node.value.upper().split()) + if sql.startswith(("ALTER TABLE ", "CREATE TABLE ", "DROP TABLE ")): + violations.append(f"{path.name}:{node.lineno}: runtime DDL") + if isinstance(node, ast.Call) and isinstance(node.func, ast.Attribute): + target = node.func.value + if node.func.attr in {"create_all", "drop_all"} or ( + node.func.attr in {"create", "drop"} + and isinstance(target, ast.Attribute) + and target.attr == "__table__" + ): + violations.append(f"{path.name}:{node.lineno}: ORM schema mutation") + assert not violations, "\n".join(violations) + + +@pytest.mark.db +def test_initialization_sql_provisions_latest_report_setting(db_session): + definition = db_session.query(ApplicationSetting).filter_by( + key="REPORT_NOTIFICATIONS_LATEST_ONLY" + ).one() + assert definition.category == "Email Automation" + assert definition.value_type == "boolean" + assert definition.default_value == "true" + assert definition.requires_restart is False diff --git a/src/backend/tests/crud/test_host_technology_locations.py b/src/backend/tests/crud/test_host_technology_locations.py index b2191ff..dc9501f 100644 --- a/src/backend/tests/crud/test_host_technology_locations.py +++ b/src/backend/tests/crud/test_host_technology_locations.py @@ -1,4 +1,4 @@ -"""MariaDB regression coverage for long technology location lists and upgrades.""" +"""MariaDB regression coverage for long locations using the current SQL schema.""" from datetime import datetime @@ -7,12 +7,11 @@ from crud.base import CRUDService from models.parser_models import Host, Institution, Scan, ScanHostTechnology -from services.schema_migrations import ensure_host_technology_location_capacity pytestmark = pytest.mark.db -def test_upgrades_existing_location_column_and_preserves_long_values(db_session): +def test_current_schema_preserves_long_location_values(db_session): institution = Institution(name="Location regression") host = Host(ip_address="192.0.2.10") db_session.add_all([institution, host]) @@ -37,16 +36,14 @@ def test_upgrades_existing_location_column_and_preserves_long_values(db_session) ) db_session.commit() engine = db_session.get_bind() - # Reproduce an existing deployment's schema, not just a newly created table. + # The initialization SQL must provide this capacity without runtime DDL. with engine.begin() as connection: - connection.execute( - text( - "ALTER TABLE scan_host_technologies MODIFY COLUMN location_name VARCHAR(1024) NULL" - ) - ) - - ensure_host_technology_location_capacity(engine) - ensure_host_technology_location_capacity(engine) + assert connection.execute(text( + "SELECT DATA_TYPE FROM information_schema.COLUMNS " + "WHERE TABLE_SCHEMA = DATABASE() " + "AND TABLE_NAME = 'scan_host_technologies' " + "AND COLUMN_NAME = 'location_name'" + )).scalar() == "longtext" db_session.expire_all() assert ( db_session.get(ScanHostTechnology, observation_id).location_name diff --git a/src/backend/tests/services/test_latest_reports.py b/src/backend/tests/services/test_latest_reports.py index 2ace610..7234278 100644 --- a/src/backend/tests/services/test_latest_reports.py +++ b/src/backend/tests/services/test_latest_reports.py @@ -16,7 +16,6 @@ from services.latest_reports import latest_notification_scans from crud.contact_persons import notify_contact_for_unnotified_scans from services.application_settings import ( - ensure_latest_report_setting, save_application_setting, latest_report_only_enabled, ) @@ -42,6 +41,14 @@ def report_series(): ], ) db = sessionmaker(bind=engine)() + # Tests provision definitions just as the initialization SQL does in deployment. + db.add(ApplicationSetting( + key="REPORT_NOTIFICATIONS_LATEST_ONLY", + label="Only notify for the latest report per scan task", + description="Skip older Greenbone report generations.", + category="Email Automation", value_type="boolean", default_value="true", + read_only=False, is_secret=False, requires_restart=False, + )) institution = Institution(name="Same target") db.add(institution) db.flush() @@ -84,11 +91,10 @@ def test_latest_generation_is_selected_per_task_not_target(report_series): assert latest_notification_scans(db, [a1, a2, b1]) == [a2, b1] -def test_policy_toggle_applies_live_and_is_not_overwritten_on_upgrade(report_series): +def test_policy_toggle_applies_live_across_workers(report_series): db, (a2, a1, b1) = report_series original = settings.REPORT_NOTIFICATIONS_LATEST_ONLY try: - ensure_latest_report_setting(db) row = ( db.query(ApplicationSetting) .filter_by(key="REPORT_NOTIFICATIONS_LATEST_ONLY") @@ -104,7 +110,6 @@ def test_policy_toggle_applies_live_and_is_not_overwritten_on_upgrade(report_ser settings.REPORT_NOTIFICATIONS_LATEST_ONLY = True assert latest_report_only_enabled(db) is False assert latest_notification_scans(db, [a1, a2, b1]) == [a1, a2, b1] - ensure_latest_report_setting(db) assert row.value == "false" assert db.query(ApplicationSetting).filter_by(key=row.key).count() == 1 save_application_setting( diff --git a/src/backend/tests/services/test_schema_migrations.py b/src/backend/tests/services/test_schema_migrations.py deleted file mode 100644 index 2e05915..0000000 --- a/src/backend/tests/services/test_schema_migrations.py +++ /dev/null @@ -1,61 +0,0 @@ -from unittest.mock import MagicMock - -import pytest -from sqlalchemy.exc import OperationalError - -from services.schema_migrations import ensure_host_technology_location_capacity - - -@pytest.mark.parametrize( - "column_type, expected_alters", - [("varchar", 1), ("text", 1), ("longtext", 0), (None, 0)], -) -def test_location_upgrade_is_idempotent(column_type, expected_alters): - engine = MagicMock() - engine.dialect.name = "mysql" - connection = engine.begin.return_value.__enter__.return_value - connection.execute.return_value.scalar.side_effect = [1, column_type] - - ensure_host_technology_location_capacity(engine) - - statements = [str(call.args[0]) for call in connection.execute.call_args_list] - assert sum(sql.startswith("ALTER TABLE") for sql in statements) == expected_alters - assert statements[0].startswith("SELECT GET_LOCK") - assert statements[-1].startswith("SELECT RELEASE_LOCK") - - -def test_location_upgrade_releases_lock_on_failure(): - engine = MagicMock() - engine.dialect.name = "mariadb" - connection = engine.begin.return_value.__enter__.return_value - locked = MagicMock() - locked.scalar.return_value = 1 - column = MagicMock() - column.scalar.return_value = "varchar" - connection.execute.side_effect = [ - locked, - column, - OperationalError("ALTER", {}, Exception("denied")), - MagicMock(), - ] - - with pytest.raises(OperationalError): - ensure_host_technology_location_capacity(engine) - assert str(connection.execute.call_args.args[0]).startswith("SELECT RELEASE_LOCK") - - -def test_location_upgrade_fails_if_lock_is_unavailable(): - engine = MagicMock() - engine.dialect.name = "mysql" - connection = engine.begin.return_value.__enter__.return_value - connection.execute.return_value.scalar.return_value = 0 - with pytest.raises(RuntimeError, match="schema upgrade lock"): - ensure_host_technology_location_capacity(engine) - assert connection.execute.call_count == 1 - - -def test_location_upgrade_skips_other_dialects(): - engine = MagicMock() - engine.dialect.name = "sqlite" - ensure_host_technology_location_capacity(engine) - engine.begin.assert_not_called()