Remediate the 2026-09-03 feature-test findings

migrations/versions/7d39_audit_retention_key_merge.py

@@ -0,0 +1,95 @@
+"""Merge the second audit-retention setting into the one that is actually read.
+
+Two keys existed for one setting. shopdb/core/services/auditretention.py reads
+`audit_retentiondays` (seeded under category 'site', default 0). A second key
+`audit_retention_days` was seeded under category 'audit' with a default of 90,
+and the Audit & Logging settings page wrote to that one - so an administrator
+set a retention period, reloaded, saw it persisted, and nothing ever consulted
+it. Retention stayed off while the page said otherwise.
+
+The page now writes the key that is read, and the duplicate is out of the seed
+list. This handles the sites that already have it: if an operator set a real
+value on the dead key and never set the live one, that intent is carried over
+rather than dropped. Their intent was "keep 30 days", and the fix should not
+silently discard it.
+
+Not carried over when the live key already holds a non-default value - that one
+was set through the page that worked, and it wins.
+
+The dead key is then deleted. Leaving it would preserve the ambiguity that
+caused this, and nothing reads it.
+
+Revision ID: 7d39_audit_retention_key_merge
+Revises: 7d38_widen_importbatch_status
+"""
+from alembic import op
+
+import sqlalchemy as sa
+
+
+revision = '7d39_audit_retention_key_merge'
+down_revision = '7d38_widen_importbatch_status'
+branch_labels = None
+depends_on = None
+
+DEAD = 'audit_retention_days'
+LIVE = 'audit_retentiondays'
+
+
+def _value(bind, key):
+    return bind.execute(
+        sa.text('SELECT value FROM settings WHERE `key` = :key'
+                if bind.dialect.name == 'mysql'
+                else 'SELECT value FROM settings WHERE "key" = :key'),
+        {'key': key}).scalar()
+
+
+def upgrade():
+    bind = op.get_bind()
+    if not sa.inspect(bind).has_table('settings'):
+        return
+
+    dead = _value(bind, DEAD)
+    if dead is None:
+        return
+
+    live = _value(bind, LIVE)
+    # '90' was the dead key's seeded default, so it is not evidence of intent.
+    operator_set_dead = dead not in (None, '', '90')
+    live_is_default = live in (None, '', '0')
+
+    if operator_set_dead and live_is_default:
+        if live is None:
+            bind.execute(sa.text(
+                'INSERT INTO settings (`key`, value, valuetype, category, '
+                'description, createddate, modifieddate, isactive) VALUES '
+                '(:key, :value, :valuetype, :category, :description, '
+                'NOW(), NOW(), 1)'
+                if bind.dialect.name == 'mysql' else
+                'INSERT INTO settings ("key", value, valuetype, category, '
+                'description, createddate, modifieddate, isactive) VALUES '
+                '(:key, :value, :valuetype, :category, :description, '
+                "datetime('now'), datetime('now'), 1)"),
+                {'key': LIVE, 'value': dead, 'valuetype': 'integer',
+                 'category': 'site',
+                 'description': 'Delete audit log entries older than this many '
+                                'days. 0 keeps everything forever.'})
+        else:
+            bind.execute(sa.text(
+                'UPDATE settings SET value = :value WHERE `key` = :key'
+                if bind.dialect.name == 'mysql' else
+                'UPDATE settings SET value = :value WHERE "key" = :key'),
+                {'value': dead, 'key': LIVE})
+
+    bind.execute(sa.text(
+        'DELETE FROM settings WHERE `key` = :key'
+        if bind.dialect.name == 'mysql' else
+        'DELETE FROM settings WHERE "key" = :key'), {'key': DEAD})
+
+
+def downgrade():
+    """Deliberately a no-op.
+
+    Re-creating a key that nothing reads, to sit beside the one that works,
+    would restore the defect. The live setting keeps whatever value it holds.
+    """

migrations/versions/7d40_machine_maintenance_dates_are_dates.py

@@ -0,0 +1,73 @@
+"""Store a maintenance date as a DATE, because that is what it is.
+
+lastmaintenancedate and nextmaintenancedate were DATETIME columns holding
+midnight. BaseModel.to_dict serialises a datetime as isoformat + 'Z', so
+2026-12-15 00:00:00 went out as "2026-12-15T00:00:00Z" and a browser west of
+UTC rendered it as 14 December. The maintenance panel showed every date one
+calendar day early on this site, and would show it a day LATE east of UTC.
+
+There is no time in a maintenance date. Nobody records that a machine was
+serviced at midnight; the form offers a date picker, the CSV importer takes
+'2026-06-01', and the value is compared against other dates. Every sibling
+already models it correctly - measuringtools.lastcalibrationdate,
+warranties.startdate and enddate are all db.Date - so this is machines being
+the odd one out rather than a new decision.
+
+As a Date, to_dict emits "2026-12-15" with no timezone marker at all, and
+nothing can shift it.
+
+MySQL truncates the (midnight) time component on the ALTER, so no data moves.
+SQLite stores what it is given and does not enforce column types, so the
+conversion is a no-op there and is skipped.
+
+Revision ID: 7d40_machine_maintenance_dates
+Revises: 7d39_audit_retention_key_merge
+"""
+from alembic import op
+
+import sqlalchemy as sa
+
+
+revision = '7d40_machine_maintenance_dates'
+down_revision = '7d39_audit_retention_key_merge'
+branch_labels = None
+depends_on = None
+
+COLUMNS = ('lastmaintenancedate', 'nextmaintenancedate')
+
+
+def _machines_table(bind):
+    inspector = sa.inspect(bind)
+    if 'machines' not in inspector.get_table_names():
+        return None
+    return {column['name'] for column in inspector.get_columns('machines')}
+
+
+def upgrade():
+    bind = op.get_bind()
+    if bind.dialect.name == 'sqlite':
+        return
+    present = _machines_table(bind)
+    if not present:
+        return
+    for column in COLUMNS:
+        if column in present:
+            op.alter_column('machines', column,
+                            existing_type=sa.DateTime(),
+                            type_=sa.Date(),
+                            existing_nullable=True)
+
+
+def downgrade():
+    bind = op.get_bind()
+    if bind.dialect.name == 'sqlite':
+        return
+    present = _machines_table(bind)
+    if not present:
+        return
+    for column in COLUMNS:
+        if column in present:
+            op.alter_column('machines', column,
+                            existing_type=sa.Date(),
+                            type_=sa.DateTime(),
+                            existing_nullable=True)

migrations/versions/7d41_drop_guest_tools_secrets.py

@@ -0,0 +1,56 @@
+"""Revoke a guest grant that never did anything, before it locks the role.
+
+tools.secrets was declared guest-grantable, so the role grid offered it and
+stored it - and the secure-send route never consulted the guest ROLE at all. It
+decides on the securesend_allow_guests setting, which fails closed and carries
+the rate limit. A site that ticked the box got no change in behaviour, in either
+direction.
+
+The plugin no longer declares it grantable. That alone would repeat the deadlock
+fixed in 723a4ff: update_role refuses the WHOLE request when a submitted
+permission is not grantable, and the role grid resubmits what the role already
+holds - so a site whose guest role holds tools.secrets could no longer save the
+guest role at all, including to change something else entirely.
+
+So the grant is removed here, from the one role that could hold it. Nothing is
+lost: it granted nothing. Guest secret-link creation stays exactly where it was,
+under securesend_allow_guests.
+
+Only the GUEST role is touched. A signed-in role holding tools.secrets is
+unaffected - that permission works normally for an account.
+
+Revision ID: 7d41_drop_guest_tools_secrets
+Revises: 7d40_machine_maintenance_dates
+"""
+from alembic import op
+
+import sqlalchemy as sa
+
+
+revision = '7d41_drop_guest_tools_secrets'
+down_revision = '7d40_machine_maintenance_dates'
+branch_labels = None
+depends_on = None
+
+
+def upgrade():
+    bind = op.get_bind()
+    inspector = sa.inspect(bind)
+    tables = set(inspector.get_table_names())
+    if not {'roles', 'permissions', 'rolepermissions'} <= tables:
+        return
+
+    bind.execute(sa.text(
+        'DELETE FROM rolepermissions WHERE roleid IN '
+        '(SELECT roleid FROM roles WHERE rolename = :guest) '
+        'AND permissionid IN '
+        '(SELECT permissionid FROM permissions WHERE name = :permission)'),
+        {'guest': 'guest', 'permission': 'tools.secrets'})
+
+
+def downgrade():
+    """Deliberately a no-op.
+
+    Re-granting a permission that nothing reads would restore the confusion
+    rather than any capability.
+    """