Qgis security scan

.github/workflows/packages.yml

@@ -148,7 +148,7 @@ jobs:
           pip3 wheel -r mergin_client.egg-info/requires.txt -w mergin/deps
           # special care for pygeodiff
           unzip mergin/deps/pygeodiff-*.whl -d mergin/deps
-          # remove unncesessary files
+          # remove unnecessary files
           rm -rf mergin/deps/*.dist-info
           rm -rf mergin/deps/*.data
           rm -rf mergin/deps/pygeodiff.libs

Mergin/clone_project_dialog.py

@@ -6,10 +6,6 @@
 from qgis.PyQt.QtWidgets import (
     QDialog,
     QDialogButtonBox,
-    QFileDialog,
-    QApplication,
-    QMessageBox,
-    QComboBox,
 )
 from qgis.PyQt.QtCore import Qt
 from qgis.PyQt import uic

Mergin/collapsible_message_box.py

@@ -2,10 +2,10 @@
 # Copyright Lutra Consulting Limited
 
 from qgis.PyQt.QtCore import Qt
-from qgis.PyQt.QtWidgets import QMessageBox
+from qgis.PyQt.QtWidgets import QMessageBox, QWidget
 
 
-class CollapsibleBox(QtWidgets.QWidget):
+class CollapsibleBox(QWidget):
     def __init__(self, text, details, title="Mergin Maps error", parent=None):
         msg = QMessageBox()
         msg.setWindowTitle(title)

Mergin/configuration_dialog.py

@@ -172,7 +172,7 @@ def test_connection(self):
                     self.ui.test_status.setText("<font color=green> OK </font>")
                     return True
 
-                self.ui.test_status.setText(f"<font color=orange>Follow the instructions in the browser...</font>")
+                self.ui.test_status.setText("<font color=orange>Follow the instructions in the browser...</font>")
                 ok, msg = test_server_connection(self.server_url(), use_sso=True, sso_email=self.get_sso_email())
 
             if url_reachable(self.server_url()):

Mergin/create_project_wizard.py

@@ -5,10 +5,9 @@
 import shutil
 from pathlib import Path
 from qgis.PyQt import uic
-from qgis.PyQt.QtCore import QSettings, Qt, QVariant, QSortFilterProxyModel
+from qgis.PyQt.QtCore import QSettings, Qt, QSortFilterProxyModel
 from qgis.PyQt.QtWidgets import (
     QAbstractItemView,
-    QApplication,
     QComboBox,
     QFileDialog,
     QHeaderView,

Mergin/data_item.py

@@ -47,7 +47,6 @@
     get_local_mergin_projects_info,
 )
 from .utils_auth import AuthTokenExpiredError
-from .mergin.merginproject import MerginProject
 
 
 class MerginRemoteProjectItem(QgsDataItem):
@@ -204,7 +203,7 @@ def remove_local_project(self):
         cur_proj_path = cur_proj.absolutePath()
         msg = (
             "Your local changes will be lost. Make sure your project is synchronised with server. \n\n"
-            "Do you want to proceed?".format(self.project_name)
+            "Do you want to proceed?"
         )
         btn_reply = QMessageBox.question(
             None,
@@ -219,10 +218,7 @@ def remove_local_project(self):
         if os.path.exists(self.path):
             try:
                 if same_dir(cur_proj_path, self.path):
-                    msg = (
-                        "The project is currently open. It will get cleared if you proceed.\n\n"
-                        "Proceed anyway?".format(self.project_name)
-                    )
+                    msg = "The project is currently open. It will get cleared if you proceed.\n\n" "Proceed anyway?"
                     btn_reply = QMessageBox.question(
                         None,
                         "Remove local project",
@@ -508,7 +504,7 @@ def fetch_more(self):
             QMessageBox.information(None, "Fetch Mergin Maps Projects", "All projects already listed.")
             return
         page_to_get = floor(len(self.projects) / PROJS_PER_PAGE) + 1
-        dummy = self.fetch_projects(page=page_to_get)
+        self.fetch_projects(page=page_to_get)
         self.refresh()
 
     def reload(self):

Mergin/diff.py

@@ -13,14 +13,11 @@
 from qgis.PyQt.QtGui import QColor
 
 from qgis.core import (
-    QgsApplication,
     QgsVectorLayer,
     QgsFeature,
     QgsGeometry,
     QgsFields,
     QgsField,
-    QgsProject,
-    QgsLayerTreeLayer,
     QgsConditionalStyle,
     QgsSymbolLayerUtils,
     QgsMarkerSymbol,
@@ -78,14 +75,22 @@ def get_row_from_db(db_conn, schema_table, entry_changes):
     Fetches a single row from DB's table based on the values of pkeys
     in changeset entry
     """
-    c = db_conn.cursor()
-    where_bits = []
-    for i, col in enumerate(schema_table.columns):
-        if col.pkey:
-            where_bits.append('"{}" = {}'.format(col.name, old_value_for_column_by_index(entry_changes, i)))
+    with db_conn.cursor() as c:
+        where_clauses = []
+        query_params = []
 
-    c.execute('SELECT * FROM "{}" WHERE {}'.format(schema_table.name, " AND ".join(where_bits)))
-    return c.fetchone()
+        for i, col in enumerate(schema_table.columns):
+            if col.pkey:
+                where_clauses.append('"{}" = %s'.format(col.name))
+                val = old_value_for_column_by_index(entry_changes, i)
+                query_params.append(val)
+
+        # We parameterize values securely; table/column names are trusted internal schema objects.
+        query = 'SELECT * FROM "{}" WHERE {}'.format(schema_table.name, " AND ".join(where_clauses))  # nosec B608
+
+        c.execute(query, tuple(query_params))
+
+        return c.fetchone()
 
 
 def parse_gpkg_geom_encoding(wkb_with_gpkg_hdr):
@@ -215,7 +220,6 @@ def diff_table_to_features(diff_table, schema_table, fields, cols_to_flds, db_co
     Input is list of tuples (type, changes) where type is 'insert'/'update'/'delete'
     and changes is a list of dicts. Each dict with 'column', 'old', 'new' (old/new optional)
     """
-    column_names = [column.name for column in schema_table.columns]
     features = []
 
     fld_geometry_idx = fields.indexOf("geometry")
@@ -225,7 +229,6 @@ def diff_table_to_features(diff_table, schema_table, fields, cols_to_flds, db_co
 
     for entry_type, entry_changes in diff_table:
         f = QgsFeature(fields)
-        row = [None for i in range(len(column_names))]
 
         f["_op"] = entry_type
 
@@ -235,7 +238,7 @@ def diff_table_to_features(diff_table, schema_table, fields, cols_to_flds, db_co
 
             for i in range(len(db_row)):
                 if i == geom_col_index:
-                    if db_row[i] == None:
+                    if db_row[i] is None:
                         continue
                     wkb = parse_gpkg_geom_encoding(db_row[i])
                     g = QgsGeometry()
@@ -254,7 +257,7 @@ def diff_table_to_features(diff_table, schema_table, fields, cols_to_flds, db_co
                 i = entry_change["column"]
                 if "old" in entry_change:
                     if i == geom_col_index:
-                        if entry_change["old"] == None:
+                        if entry_change["old"] is None:
                             # Empty geometry
                             continue
                         wkb_with_gpkg_hdr = base64.decodebytes(entry_change["old"].encode("ascii"))
@@ -278,7 +281,7 @@ def diff_table_to_features(diff_table, schema_table, fields, cols_to_flds, db_co
                 value = "?"
 
             if i == geom_col_index:
-                if value == None:
+                if value is None:
                     # Empty geometry
                     continue
                 wkb_with_gpkg_hdr = base64.decodebytes(value.encode("ascii"))
@@ -357,7 +360,7 @@ def make_local_changes_layer(mp, layer):
         "memory",
     )
     if not vl.isValid():
-        return None, f"Failed to create memory layer for local changes"
+        return None, "Failed to create memory layer for local changes"
 
     vl.dataProvider().addAttributes(fields)
     vl.updateFields()
@@ -443,7 +446,7 @@ def get_layer_geometry_info(schema_json, table_name):
 
 def style_diff_layer(layer, schema_table):
     """Apply conditional styling and symbology to diff layer"""
-    ### setup conditional styles!
+    # setup conditional styles!
     st = layer.conditionalStyles()
     color_red = QColor("#ffdce0")
     color_green = QColor("#dcffe4")

Mergin/diff_dialog.py

@@ -6,7 +6,7 @@
 from qgis.PyQt import uic
 from qgis.PyQt.QtCore import Qt, QSettings
 from qgis.PyQt.QtGui import QIcon, QColor
-from qgis.PyQt.QtWidgets import QDialog, QPushButton, QDialogButtonBox, QMenu, QAction
+from qgis.PyQt.QtWidgets import QDialog, QPushButton, QMenu, QAction
 from qgis.core import (
     QgsProject,
     QgsVectorLayerCache,
@@ -15,7 +15,6 @@
     QgsMessageLog,
     Qgis,
     QgsApplication,
-    QgsWkbTypes,
 )
 from qgis.gui import (
     QgsGui,

Mergin/plugin.py

@@ -3,9 +3,9 @@
 # GPLv3 license
 # Copyright Lutra Consulting Limited
 try:
-    import sip
+    import sip  # noqa: F401
 except ImportError:
-    from PyQt6 import sip
+    pass
 import os
 from functools import partial
 from qgis.PyQt.QtCore import QUrl, QSettings, Qt
@@ -351,7 +351,7 @@ def configure_db_sync(self):
         mp = MerginProject(project_path)
         try:
             project_name = mp.project_full_name()
-        except InvalidProject as e:
+        except InvalidProject:
             iface.messageBar().pushMessage(
                 "Mergin",
                 "Current project is not a Mergin project. Please open a Mergin project first.",
@@ -401,7 +401,7 @@ def set_current_workspace(self, workspace):
             # check action required flag
             try:
                 service_response = self.mc.workspace_service(workspace_id)
-            except ClientError as e:
+            except ClientError:
                 return
             except AuthTokenExpiredError:
                 self.auth_token_expired()
@@ -492,16 +492,16 @@ def find_project(self):
         try:
             workspaces = self.mc.workspaces_list()
             dlg.enable_workspace_switching(len(workspaces) > 1)
-        except:
-            pass
+        except ClientError:
+            dlg.enable_workspace_switching(False)
 
         dlg.exec()
 
     def switch_workspace(self):
         """Open new Switch workspace dialog"""
         try:
             workspaces = self.mc.workspaces_list()
-        except (URLError, ClientError) as e:
+        except (URLError, ClientError):
             return  # Server does not support workspaces
         except AuthTokenExpiredError:
             self.auth_token_expired()
@@ -547,9 +547,9 @@ def add_context_menu_actions(self, layers):
                 "arcgisvectortileservice",
                 "vtpkvectortiles",
             )
-        for l in layers:
-            if l.dataProvider().name() in provider_names:
-                self.iface.addCustomActionForLayer(self.action_export_mbtiles, l)
+        for layer in layers:
+            if layer.dataProvider().name() in provider_names:
+                self.iface.addCustomActionForLayer(self.action_export_mbtiles, layer)
 
     def unload(self):
         from .utils import pygeodiff

Mergin/processing/algs/create_diff.py

@@ -6,16 +6,13 @@
 
 import os
 import sqlite3
-import shutil
 
 from qgis.PyQt.QtGui import QIcon
 from qgis.core import (
     QgsFeatureSink,
-    QgsProcessing,
     QgsProcessingUtils,
     QgsProcessingException,
     QgsProcessingAlgorithm,
-    QgsProcessingContext,
     QgsProcessingParameterFile,
     QgsProcessingParameterNumber,
     QgsProcessingParameterVectorLayer,
@@ -25,7 +22,6 @@
 from ..postprocessors import StylingPostProcessor
 
 from ...mergin.merginproject import MerginProject
-from ...mergin.utils import get_versions_with_file_changes
 from ...mergin.deps import pygeodiff
 
 from ...diff import (
@@ -144,7 +140,7 @@ def processAlgorithm(self, parameters, context, feedback):
         except (ClientError, ValueError) as e:
             raise QgsProcessingException(f"Error creating Mergin Maps client: {e}")
 
-        mp = MerginProject(project_dir)
+        mp = MerginProject(project_dir)  # noqa: F841
 
         feedback.pushInfo("Downloading base file…")
         base_file = QgsProcessingUtils.generateTempFilename(file_name)
@@ -189,7 +185,6 @@ def processAlgorithm(self, parameters, context, feedback):
             features = diff_table_to_features(diff[table_name], db_schema[table_name], fields, fields_mapping, db_conn)
             feedback.setProgress(40)
 
-            current = 40
             step = 60.0 / len(features) if features else 0
             for i, f in enumerate(features):
                 if feedback.isCanceled():

Mergin/processing/algs/create_report.py

@@ -2,8 +2,6 @@
 
 from qgis.PyQt.QtGui import QIcon
 from qgis.core import (
-    QgsVectorFileWriter,
-    QgsProcessing,
     QgsProcessingException,
     QgsProcessingAlgorithm,
     QgsProcessingContext,

Mergin/processing/algs/download_vector_tiles.py

@@ -18,7 +18,6 @@
     QgsCsException,
     QgsCoordinateReferenceSystem,
     QgsBlockingNetworkRequest,
-    QgsSqliteUtils,
     QgsDataSourceUri,
     QgsVectorTileLayer,
     QgsCoordinateTransform,
@@ -218,9 +217,9 @@ def processAlgorithm(self, parameters, context, feedback):
                 f"{wgs_extent.xMinimum()},{wgs_extent.yMinimum()},{wgs_extent.xMaximum()},{wgs_extent.yMaximum()}"
             )
             writer.set_metadata_value("bounds", bounds_str)
-        except QgsCsException as e:
+        except QgsCsException:
             pass
-        except AttributeError as e:
+        except AttributeError:
             pass
 
         step_feedback = QgsProcessingMultiStepFeedback(self.max_zoom + 1, feedback)
@@ -284,7 +283,7 @@ def postProcessAlgorithm(self, context, feedback):
         tile_layer = QgsVectorTileLayer(bytes(ds_uri.encodedUri()).decode(), name)
         if tile_layer.isValid():
             if context.project():
-                err = tile_layer.importNamedStyle(self.style_document)
+                tile_layer.importNamedStyle(self.style_document)
                 metadata = tile_layer.metadata()
                 metadata.setRights(self.attribution)
                 tile_layer.setMetadata(metadata)

Mergin/processing/algs/extract_local_changes.py

@@ -10,10 +10,8 @@
 from qgis.PyQt.QtGui import QIcon
 from qgis.core import (
     QgsFeatureSink,
-    QgsProcessing,
     QgsProcessingException,
     QgsProcessingAlgorithm,
-    QgsProcessingContext,
     QgsProcessingParameterFile,
     QgsProcessingParameterVectorLayer,
     QgsProcessingParameterFeatureSink,
@@ -122,7 +120,6 @@ def processAlgorithm(self, parameters, context, feedback):
             features = diff_table_to_features(diff[table_name], db_schema[table_name], fields, fields_mapping, db_conn)
             feedback.setProgress(20)
 
-            current = 20
             step = 80.0 / len(features) if features else 0
             for i, f in enumerate(features):
                 if feedback.isCanceled():

Mergin/processing/provider.py

@@ -4,8 +4,6 @@
 # Copyright Lutra Consulting Limited
 
 
-import os
-
 from qgis.PyQt.QtGui import QIcon
 
 from qgis.core import QgsProcessingProvider