From 516d67a8c51d78bc86426d6661ad0a28f8340639 Mon Sep 17 00:00:00 2001 From: LunarCapsule127 Date: Sat, 29 Aug 2026 21:29:21 +0100 Subject: [PATCH 1/6] Update __manifest__.py --- spp_attachment_av_scan/__manifest__.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) 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", From c4db10dc96a72d70bdb9d7a00499bce7da0f582b Mon Sep 17 00:00:00 2001 From: LunarCapsule127 Date: Sat, 29 Aug 2026 21:30:06 +0100 Subject: [PATCH 2/6] Update quarantine_cron.xml --- spp_attachment_av_scan/data/quarantine_cron.xml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) 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 From d9998848a2a9aed906081ab8a7143b66da2fe9a2 Mon Sep 17 00:00:00 2001 From: LunarCapsule127 Date: Sat, 29 Aug 2026 21:31:23 +0100 Subject: [PATCH 3/6] Create post-migrate.py --- .../migrations/19.0.2.2.0/post-migrate.py | 36 +++++++++++++++++++ 1 file changed, 36 insertions(+) create mode 100644 spp_attachment_av_scan/migrations/19.0.2.2.0/post-migrate.py 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..b73eb382 --- /dev/null +++ b/spp_attachment_av_scan/migrations/19.0.2.2.0/post-migrate.py @@ -0,0 +1,36 @@ +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): + """Protect admin-tuned quarantine crons/params from upgrade resets. + + The records in ``data/quarantine_cron.xml`` are now declared + ``noupdate="1"``, but that flag is only honored when a record is first + created. On any database that installed this module before the flag was + added, the ``ir.model.data`` rows already exist with ``noupdate = False``, + so every upgrade keeps rewriting them to the shipped defaults. Flip the + flag on the existing rows; leave the stored values untouched so an admin's + tuning survives 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, + ) From 0242d5428a7d5c8d27c7d98bac1e5062cd9a7fc6 Mon Sep 17 00:00:00 2001 From: LunarCapsule127 Date: Sat, 29 Aug 2026 21:36:29 +0100 Subject: [PATCH 4/6] Update test_pending_scan_sweep.py --- spp_attachment_av_scan/tests/test_pending_scan_sweep.py | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/spp_attachment_av_scan/tests/test_pending_scan_sweep.py b/spp_attachment_av_scan/tests/test_pending_scan_sweep.py index 929d7b58..0f23ba87 100644 --- a/spp_attachment_av_scan/tests/test_pending_scan_sweep.py +++ b/spp_attachment_av_scan/tests/test_pending_scan_sweep.py @@ -401,6 +401,10 @@ def test_the_cron_and_config_defaults_are_not_reset_by_a_module_upgrade(self): "config_param_pending_sweep_min_age_minutes", "config_param_pending_sweep_batch_size", "config_param_pending_sweep_max_attempts", + "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)]) From 48f78fb5e19d609b76825ded9502db41a05e615e Mon Sep 17 00:00:00 2001 From: LunarCapsule127 Date: Sun, 6 Sep 2026 22:22:41 +0100 Subject: [PATCH 5/6] fix(spp_attachment_av_scan): address review on quarantine noupdate - add the missing 19.0.2.2.0 changelog entry to readme/HISTORY.md and README.rst, so the shipped module no longer advertises a version its published changelog has no record of - correct the post-migration docstring: is read off the file by _tag_root, so it already stops the rewrite on installed databases. The migration reconciles the ir_model_data.noupdate column, which the xmlid upsert never touches; no pre-migrate is needed and the XML-only fix in #470 is correct as it stands - move the quarantine records' upgrade guard out of the pending-sweep test into TestEncryptedQuarantine, alongside the crons it already covers --- spp_attachment_av_scan/README.rst | 13 +++++++++ .../migrations/19.0.2.2.0/post-migrate.py | 27 +++++++++++++------ spp_attachment_av_scan/readme/HISTORY.md | 11 ++++++++ .../tests/test_ir_attachment.py | 20 ++++++++++++++ .../tests/test_pending_scan_sweep.py | 4 --- 5 files changed, 63 insertions(+), 12 deletions(-) diff --git a/spp_attachment_av_scan/README.rst b/spp_attachment_av_scan/README.rst index fbb3956f..ed8ae681 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/migrations/19.0.2.2.0/post-migrate.py b/spp_attachment_av_scan/migrations/19.0.2.2.0/post-migrate.py index b73eb382..3edfa0c7 100644 --- 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 @@ -11,15 +11,26 @@ def migrate(cr, version): - """Protect admin-tuned quarantine crons/params from upgrade resets. + """Reconcile ``ir_model_data.noupdate`` for the quarantine crons/params. - The records in ``data/quarantine_cron.xml`` are now declared - ``noupdate="1"``, but that flag is only honored when a record is first - created. On any database that installed this module before the flag was - added, the ``ir.model.data`` rows already exist with ``noupdate = False``, - so every upgrade keeps rewriting them to the shipped defaults. Flip the - flag on the existing rows; leave the stored values untouched so an admin's - tuning survives and untouched defaults stay as shipped. + ``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( """ 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/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.""" diff --git a/spp_attachment_av_scan/tests/test_pending_scan_sweep.py b/spp_attachment_av_scan/tests/test_pending_scan_sweep.py index 0f23ba87..929d7b58 100644 --- a/spp_attachment_av_scan/tests/test_pending_scan_sweep.py +++ b/spp_attachment_av_scan/tests/test_pending_scan_sweep.py @@ -401,10 +401,6 @@ def test_the_cron_and_config_defaults_are_not_reset_by_a_module_upgrade(self): "config_param_pending_sweep_min_age_minutes", "config_param_pending_sweep_batch_size", "config_param_pending_sweep_max_attempts", - "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)]) From 63020263dda79977671454058ae2a653b53eb2bf Mon Sep 17 00:00:00 2001 From: LunarCapsule127 Date: Sun, 6 Sep 2026 23:39:04 +0100 Subject: [PATCH 6/6] docs(spp_attachment_av_scan): regenerate README for the 19.0.2.2.0 entry oca-gen-addon-readme renders README.rst and static/description/index.html from readme/*.md. Adding the 19.0.2.2.0 changelog fragment left both generated files stale, which failed the pre-commit job. Note the hook is not opt-in as intended: `manual: true` is not a valid pre-commit key, so it is ignored with a warning and the hook runs in the default stage. --- spp_attachment_av_scan/README.rst | 6 +++--- .../static/description/index.html | 20 ++++++++++++++++--- 2 files changed, 20 insertions(+), 6 deletions(-) diff --git a/spp_attachment_av_scan/README.rst b/spp_attachment_av_scan/README.rst index ed8ae681..87adcef3 100644 --- a/spp_attachment_av_scan/README.rst +++ b/spp_attachment_av_scan/README.rst @@ -140,9 +140,9 @@ Changelog - 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 + 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 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