diff --git a/spp_attachment_av_scan/README.rst b/spp_attachment_av_scan/README.rst index fbb3956f..87adcef3 100644 --- a/spp_attachment_av_scan/README.rst +++ b/spp_attachment_av_scan/README.rst @@ -134,6 +134,19 @@ External: ``pyclamd`` (Python library for ClamAV integration) Changelog ========= +19.0.2.2.0 +~~~~~~~~~~ + +- fix: stop module upgrades from resetting the quarantine crons and + config parameters. ``data/quarantine_cron.xml`` shipped in a plain + ```` block, so every upgrade rewrote all four records to the + shipped defaults: a tuned ``quarantine_retention_days``, a changed + cron interval, or a deliberately disabled cron was silently reverted. + The file is now ``noupdate="1"``. A post-migration reconciles the + ``noupdate`` column on the ``ir_model_data`` rows of databases that + installed the module earlier, leaving the stored values untouched, so + tuned values survive and untouched defaults stay as shipped + 19.0.2.1.0 ~~~~~~~~~~ diff --git a/spp_attachment_av_scan/__manifest__.py b/spp_attachment_av_scan/__manifest__.py index d8f6caa2..0da47c30 100644 --- a/spp_attachment_av_scan/__manifest__.py +++ b/spp_attachment_av_scan/__manifest__.py @@ -1,7 +1,7 @@ { # pylint: disable=pointless-statement "name": "OpenSPP Attachment Antivirus Scan", "category": "OpenSPP", - "version": "19.0.2.1.0", + "version": "19.0.2.2.0", "sequence": 1, "author": "OpenSPP.org", "website": "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/OpenSPP/OpenSPP2", diff --git a/spp_attachment_av_scan/data/quarantine_cron.xml b/spp_attachment_av_scan/data/quarantine_cron.xml index 7dfec792..ef0393bd 100644 --- a/spp_attachment_av_scan/data/quarantine_cron.xml +++ b/spp_attachment_av_scan/data/quarantine_cron.xml @@ -1,5 +1,5 @@ - + Purge Old Quarantined Files diff --git a/spp_attachment_av_scan/migrations/19.0.2.2.0/post-migrate.py b/spp_attachment_av_scan/migrations/19.0.2.2.0/post-migrate.py new file mode 100644 index 00000000..3edfa0c7 --- /dev/null +++ b/spp_attachment_av_scan/migrations/19.0.2.2.0/post-migrate.py @@ -0,0 +1,47 @@ +import logging + +_logger = logging.getLogger(__name__) + +_RECORDS = ( + "ir_cron_purge_quarantined_files", + "ir_cron_cleanup_forensic_downloads", + "config_param_quarantine_retention_days", + "config_param_forensic_download_retention_hours", +) + + +def migrate(cr, version): + """Reconcile ``ir_model_data.noupdate`` for the quarantine crons/params. + + ``data/quarantine_cron.xml`` is declared ``noupdate="1"`` now, and on an + already-installed database that file-level flag is by itself enough to stop + the reset: ``xml_import._tag_record`` tests ``self.noupdate``, which + ``_tag_root`` reads off the ```` element rather than off the stored + row, and returns before ``_load_records`` whenever the mode is not ``init``. + So this migration is not what fixes the upgrade rewrite, and no pre-migrate + is needed; the XML-only fix applied to ``scan_sweep_cron.xml`` in #470 is + correct as it stands. + + What the XML cannot do is correct the stored rows. + ``_build_update_xmlids_query`` upserts ``ON CONFLICT ... DO UPDATE SET + (model, res_id, write_date)`` and never writes ``noupdate``, so every + database that already carries these xml_ids keeps ``noupdate = false`` in + ``ir_model_data``. That column is the one ``ir.model.data._process_end`` + consults, and the one an admin -- or the regression test for these records + -- reads to confirm they are protected. Flip it once here, and leave the + stored values alone: tuned values survive and untouched defaults stay as + shipped. + """ + cr.execute( + """ + UPDATE ir_model_data + SET noupdate = TRUE + WHERE module = 'spp_attachment_av_scan' + AND name IN %s + """, + (_RECORDS,), + ) + _logger.info( + "spp_attachment_av_scan: set noupdate on %s quarantine cron/param records", + cr.rowcount, + ) diff --git a/spp_attachment_av_scan/readme/HISTORY.md b/spp_attachment_av_scan/readme/HISTORY.md index 082eb726..4343b571 100644 --- a/spp_attachment_av_scan/readme/HISTORY.md +++ b/spp_attachment_av_scan/readme/HISTORY.md @@ -1,3 +1,14 @@ +### 19.0.2.2.0 + +- fix: stop module upgrades from resetting the quarantine crons and config + parameters. `data/quarantine_cron.xml` shipped in a plain `` block, so + every upgrade rewrote all four records to the shipped defaults: a tuned + `quarantine_retention_days`, a changed cron interval, or a deliberately + disabled cron was silently reverted. The file is now `noupdate="1"`. A + post-migration reconciles the `noupdate` column on the `ir_model_data` rows of + databases that installed the module earlier, leaving the stored values + untouched, so tuned values survive and untouched defaults stay as shipped + ### 19.0.2.1.0 - feat: sweep attachments stranded at `scan_status = pending`. Queueing a scan is diff --git a/spp_attachment_av_scan/static/description/index.html b/spp_attachment_av_scan/static/description/index.html index 0f9e0604..19c5c3b0 100644 --- a/spp_attachment_av_scan/static/description/index.html +++ b/spp_attachment_av_scan/static/description/index.html @@ -515,6 +515,20 @@

Changelog

+

19.0.2.2.0

+
    +
  • fix: stop module upgrades from resetting the quarantine crons and +config parameters. data/quarantine_cron.xml shipped in a plain +<odoo> block, so every upgrade rewrote all four records to the +shipped defaults: a tuned quarantine_retention_days, a changed +cron interval, or a deliberately disabled cron was silently reverted. +The file is now noupdate="1". A post-migration reconciles the +noupdate column on the ir_model_data rows of databases that +installed the module earlier, leaving the stored values untouched, so +tuned values survive and untouched defaults stay as shipped
    • +
+
+

19.0.2.1.0

  • feat: sweep attachments stranded at scan_status = pending. @@ -532,7 +546,7 @@

    19.0.2.1.0

    are out of scope
-
+

19.0.2.0.2

  • fix: never enqueue a malware scan while the registry is still loading. @@ -544,7 +558,7 @@

    19.0.2.0.2

    the scan status, so changed bytes never keep a stale clean verdict
-
+

19.0.2.0.1

  • fix: re-raise database errors (psycopg2.Error, @@ -554,7 +568,7 @@

    19.0.2.0.1

    unrelated downstream code
-
+

19.0.2.0.0

  • Initial migration to OpenSPP2
    • diff --git a/spp_attachment_av_scan/tests/test_ir_attachment.py b/spp_attachment_av_scan/tests/test_ir_attachment.py index a680ce96..8ff7e061 100644 --- a/spp_attachment_av_scan/tests/test_ir_attachment.py +++ b/spp_attachment_av_scan/tests/test_ir_attachment.py @@ -618,6 +618,26 @@ def test_cron_cleanup_keeps_recent_forensic_downloads(self): # Attachment should still exist (it's recent) self.assertTrue(self.attachment_model.browse(attachment_id).exists()) + def test_the_quarantine_crons_and_params_are_not_reset_by_a_module_upgrade(self): + """An upgrade must not overwrite an admin's tuning of these records. + + ``data/quarantine_cron.xml`` shipped in a plain ```` block, so every + upgrade rewrote all four records to the shipped defaults: a changed retention + window, a retimed cron, or a deliberately disabled cron was silently reverted. + The file is ``noupdate="1"`` now, and 19.0.2.2.0's post-migration flips the + stored flag on databases that predate it. + """ + for name in ( + "ir_cron_purge_quarantined_files", + "ir_cron_cleanup_forensic_downloads", + "config_param_quarantine_retention_days", + "config_param_forensic_download_retention_hours", + ): + with self.subTest(record=name): + imd = self.env["ir.model.data"].search([("module", "=", "spp_attachment_av_scan"), ("name", "=", name)]) + self.assertTrue(imd, "the record must exist") + self.assertTrue(imd.noupdate, "an upgrade must keep admin-tuned values") + @patch("odoo.addons.spp_attachment_av_scan.models.av_scanner_backend.pyclamd") def test_large_file_skipped_during_scan(self, mock_pyclamd): """Test that files exceeding max_file_size_mb are skipped."""