Backport master to dev branch

CITATION.cff

@@ -0,0 +1,9 @@
+cff-version: 1.2.0
+message: "If you use this software, please cite it as below."
+title: "Mergin Maps Server"
+type: software
+authors:
+  - name: "Lutra Consulting Ltd."
+url: "https://merginmaps.com"
+repository-code: "https://github.com/MerginMaps/server"
+license: AGPL-3.0-only

deployment/community/.env.template

@@ -121,6 +121,8 @@ LOCAL_PROJECTS=/data
 
 #MAX_CHUNK_SIZE=10 * 1024 * 1024  # 10485760 in bytes
 
+#MAX_DIFFABLE_FORCE_UPDATE_SIZE=512 * 1024 * 1024  # 536870912 in bytes - max size of an uploaded full .gpkg for which server tries to construct a diff on force update, above this it falls back to a plain full-file force update
+
 # data download
 
 #MAX_DOWNLOAD_ARCHIVE_SIZE=1024 * 1024 * 1024 * 10  # max total files size in bytes for archive download - 10 GB

deployment/enterprise/.env.template

@@ -118,6 +118,8 @@ LOCAL_PROJECTS=/data
 
 #MAX_CHUNK_SIZE=10 * 1024 * 1024  # 10485760 in bytes
 
+#MAX_DIFFABLE_FORCE_UPDATE_SIZE=512 * 1024 * 1024  # 536870912 in bytes - max size of an uploaded full .gpkg for which server tries to construct a diff on force update, above this it falls back to a plain full-file force update
+
 # data download
 
 #MAX_DOWNLOAD_ARCHIVE_SIZE=1024 * 1024 * 1024 * 10  # max total files size in bytes for archive download

development.md

@@ -21,6 +21,7 @@ $ cd server
 # Install dependencies with pipenv
 # Note: You can append --three flag in older versions of pipenv (< 3.16.8 2023-02-04)
 $ pipenv install --dev
+$ pipenv install --categories="telemetry
 $ pipenv run pre-commit install
 $ pipenv run pre-commit run --all-files
 $ export FLASK_APP=application; export COLLECT_STATISTICS=0

server/.pre-commit-config.yaml

@@ -4,4 +4,4 @@ repos:
     rev: 25.1.0
     hooks:
       - id: black
-        language_version: python3.10
+        language_version: python3.12

server/mergin/auth/config.py

@@ -2,7 +2,7 @@
 #
 # SPDX-License-Identifier: AGPL-3.0-only OR LicenseRef-MerginMaps-Commercial
 
-from decouple import config
+from decouple import config, Csv
 
 
 class Configuration(object):
@@ -19,3 +19,9 @@ class Configuration(object):
     LOCKOUT_POLICY = config("LOCKOUT_POLICY", default="5:300,10:3600")
     # trailing window in seconds over which failed login attempts are counted
     LOCKOUT_WINDOW = config("LOCKOUT_WINDOW", default=3600, cast=int)
+    # comma-separated substrings (case-insensitive) of user agents excluded from login history logging
+    LOGIN_HISTORY_EXCLUDED_USER_AGENTS = config(
+        "LOGIN_HISTORY_EXCLUDED_USER_AGENTS",
+        default="media-sync,work-packages,DB-sync",
+        cast=Csv(),
+    )

server/mergin/auth/models.py

@@ -111,6 +111,9 @@ def record_failed_login(self) -> Optional[int]:
         counting only failed attempts within the trailing LOCKOUT_WINDOW and
         since the last successful login (whichever bound is more recent).
 
+        Note: failed attempts from a user agent excluded from login_history logging
+        are not recorded at all, so they do not count toward lockout either.
+
         Returns the lockout duration in seconds if a new lock was just applied, else None.
         """
         LoginHistory.add_record(self.id, request, successful=False)
@@ -369,9 +372,20 @@ def __init__(
         self.successful = successful
         self.timestamp = datetime.datetime.now(tz=datetime.timezone.utc)
 
+    @staticmethod
+    def is_excluded_user_agent(ua: Optional[str]) -> bool:
+        """Return True if the user agent matches one of the configured exclusions
+        and should not be logged in the login history."""
+        if not ua:
+            return False
+        excluded = current_app.config.get("LOGIN_HISTORY_EXCLUDED_USER_AGENTS", [])
+        return any(pattern.lower() in ua.lower() for pattern in excluded)
+
     @staticmethod
     def add_record(user_id: int, req: request, successful: bool = True) -> None:
         ua = get_user_agent(req)
+        if LoginHistory.is_excluded_user_agent(ua):
+            return
         ip = get_ip(req)
         device_id = get_device_id(req)
         lh = LoginHistory(user_id, ua, ip, device_id, successful=successful)

server/mergin/sync/config.py

@@ -88,3 +88,7 @@ class Configuration(object):
     )
     # max batch size for fetch projects in batch endpoint
     MAX_BATCH_SIZE = config("MAX_BATCH_SIZE", default=100, cast=int)
+    # max size (in bytes) of an uploaded full .gpkg file for which server will try to construct a diff
+    MAX_DIFFABLE_FORCE_UPDATE_SIZE = config(
+        "MAX_DIFFABLE_FORCE_UPDATE_SIZE", default=512 * 1024 * 1024, cast=int
+    )

server/mergin/sync/files.py

@@ -227,6 +227,12 @@ def validate(self, data, **kwargs):
                     f"Unsupported file type detected: '{file_path}'. "
                     f"Please remove the file or try compressing it into a ZIP file before uploading.",
                 )
+
+            diff = file.get("diff")
+            if diff and not is_valid_path(diff["path"]):
+                raise ValidationError(
+                    f"Unsupported file name detected: '{diff['path']}'. Please remove the invalid characters."
+                )
         # new checks must restrict only new files not to block existing projects
         for file in data["added"]:
             file_path = file["path"]

server/mergin/sync/models.py

@@ -2163,7 +2163,11 @@ def process_chunks(
                             errors[f.path] = (
                                 f"{FileSyncErrorType.SYNC_ERROR.value}: project {self.project.workspace.name}/{self.project.name}, {result.value}"
                             )
-                    else:
+                    elif (
+                        expected_size
+                        <= current_app.config["MAX_DIFFABLE_FORCE_UPDATE_SIZE"]
+                    ):
+                        # gpkg small enough - try to construct diff server-side
                         diff_name = mergin_secure_filename(
                             f.path + "-diff-" + str(uuid.uuid4())
                         )
@@ -2188,6 +2192,12 @@ def process_chunks(
                             logging.warning(
                                 f"Geodiff: create changeset error {result.value}"
                             )
+                    else:
+                        # gpkg too large - skip diff construction and keep it as a plain force update
+                        logging.info(
+                            f"Skipping diff construction for {f.path} in project {project_path}: "
+                            f"file size {expected_size} exceeds MAX_DIFFABLE_FORCE_UPDATE_SIZE"
+                        )
         return file_changes, errors
 
 

server/mergin/sync/public_api_v2_controller.py

@@ -234,10 +234,6 @@ def create_project_version(id):
     v_next_version = ProjectVersion.to_v_name(next_version)
     version_dir = os.path.join(project.storage.project_dir, v_next_version)
 
-    pv = project.get_latest_version()
-    if pv and pv.name != version:
-        return ProjectVersionExists(version, pv.name).response(409)
-
     try:
         ChangesSchema().validate(changes)
         upload_changes = ChangesSchema().dump(changes)
@@ -283,6 +279,10 @@ def create_project_version(id):
     if requested_storage > project.workspace.storage:
         return StorageLimitHit(current_usage, project.workspace.storage).response(422)
 
+    pv = project.get_latest_version()
+    if pv and pv.name != version:
+        return ProjectVersionExists(version, pv.name).response(409)
+
     # we have done all checks but this request is just a dry-run
     if request.json.get("check_only", False):
         return NoContent, 204

server/mergin/sync/storages/disk.py

@@ -407,13 +407,19 @@ def restore_versioned_file(self, file: str, version: int):
         if not (base_meta and diffs):
             return
 
+        diffs_size = sum(os.path.getsize(d.abs_path) for d in diffs)
+        logging.info(
+            f"restore_versioned_file: file={file} target_version={version} "
+            f"basefile={base_meta.abs_path} basefile_size={base_meta.size} "
+            f"diffs={len(diffs)} diffs_total_size={diffs_size}"
+        )
+
         start = time.time()
         with self.geodiff_copy(base_meta.abs_path) as restored_file:
             copy_time = time.time() - start
             logging.info(
                 f"Restore file: {base_meta.abs_path} copied to {restored_file} in {copy_time} s"
             )
-            logging.info(f"Restoring gpkg file with {len(diffs)} diffs")
             try:
                 self.flush_geodiff_logger()  # clean geodiff logger
                 changeset = os.path.join(

server/mergin/sync/tasks.py

@@ -121,6 +121,12 @@ def create_project_version_zip(version_id: int):
     if not project_version:
         return
 
+    total_files = len(project_version.files)
+    logging.info(
+        f"create_project_version_zip: project_id={project_version.project_id} version={project_version.name} "
+        f"files={total_files} project_size={project_version.project_size}"
+    )
+
     zip_path = project_version.zip_path + ".partial"
     if os.path.exists(zip_path):
         mtime = datetime.fromtimestamp(os.path.getmtime(zip_path), tz=timezone.utc)
@@ -151,6 +157,9 @@ def create_project_version_zip(version_id: int):
                 )
         # move zip file to final location
         os.rename(zip_path, project_version.zip_path)
+        logging.info(
+            f"create_project_version_zip: finished project_id={project_version.project_id} version={project_version.name}"
+        )
     finally:
         # remove partial zip file if exists
         if os.path.exists(zip_path):

server/mergin/tests/test_auth.py

@@ -835,11 +835,33 @@ def test_api_login(client, data, headers, expected):
         assert user.last_signed_in == login_history.timestamp
 
 
+@pytest.mark.parametrize(
+    "ua", ["DB-sync/0.1", "media-sync/1.0", "work-packages-agent/2.0", "db-sync/0.1"]
+)
+def test_api_login_excluded_user_agent(client, ua):
+    """Logins from user agents on the LOGIN_HISTORY_EXCLUDED_USER_AGENTS list (matched
+    case-insensitively as a substring) are not recorded in LoginHistory"""
+    with patch("mergin.auth.models.get_user_agent") as mock:
+        mock.return_value = ua
+        user_before = User.query.filter_by(username=DEFAULT_USER[0]).first()
+        last_signed_in_before = user_before.last_signed_in
+        resp = client.post(
+            "/v1/auth/login",
+            data=json.dumps({"login": "mergin", "password": "ilovemergin"}),
+            headers=json_headers,
+        )
+        assert resp.status_code == 200
+        user = User.query.filter_by(username=DEFAULT_USER[0]).first()
+        login_history = LoginHistory.query.filter_by(user_id=user.id).first()
+        assert login_history is None
+        assert user.last_signed_in == last_signed_in_before
+
+
 def test_api_login_from_urllib(client):
-    """DB-sync logins are recorded in LoginHistory just like any other client,
+    """Non-excluded clients are recorded in LoginHistory just like any other client,
     to keep a full picture of login activity (including for lockout purposes)."""
     with patch("mergin.auth.models.get_user_agent") as mock:
-        mock.return_value = "DB-sync/0.1"
+        mock.return_value = "python-urllib/3.9"
         resp = client.post(
             "/v1/auth/login",
             data=json.dumps({"login": "mergin", "password": "ilovemergin"}),

server/mergin/tests/test_project_controller.py

@@ -1755,6 +1755,102 @@ def copy_file_failing_for_geodiff(src, dest):
     assert "diff" not in updated_file
 
 
+def test_push_force_update_size_limit(client):
+    """Server should only try to construct a diff for a force-updated (full gpkg,
+    no diff sent) upload when its size is within MAX_DIFFABLE_FORCE_UPDATE_SIZE;
+    above the limit it should skip diff construction and keep it as a plain
+    force update."""
+    working_dir = os.path.join(TMP_DIR, "test_push_force_update_size_limit")
+    # cleanup
+    if os.path.exists(working_dir):
+        shutil.rmtree(working_dir)
+
+    shutil.copytree(test_project_dir, working_dir)
+    # mimic base.gpkg was updated with inserted_1_A.gpkg (but no diff is created)
+    shutil.copy(
+        os.path.join(working_dir, "inserted_1_A.gpkg"),
+        os.path.join(working_dir, "base.gpkg"),
+    )
+    base_gpkg_size = os.path.getsize(os.path.join(working_dir, "base.gpkg"))
+    changes = {
+        "added": [],
+        "removed": [],
+        "updated": [
+            file_info(working_dir, "base.gpkg", chunk_size=CHUNK_SIZE),
+            file_info(working_dir, "test.txt", chunk_size=CHUNK_SIZE),
+        ],
+    }
+
+    # below limit -> diff is still constructed server-side
+    upload, upload_dir = create_transaction("mergin", changes)
+    upload_chunks(upload_dir, upload.changes, src_dir=working_dir)
+    with patch.dict(
+        client.application.config,
+        {"MAX_DIFFABLE_FORCE_UPDATE_SIZE": base_gpkg_size + 1},
+    ):
+        resp = client.post(f"/v1/project/push/finish/{upload.transaction_id}")
+    assert resp.status_code == 200
+    latest_version = upload.project.get_latest_version()
+    assert (
+        latest_version.changes.filter(
+            FileHistory.change == PushChangeType.UPDATE.value
+        ).count()
+        == 1
+    )
+    assert (
+        latest_version.changes.filter(
+            FileHistory.change == PushChangeType.UPDATE_DIFF.value
+        ).count()
+        == 1
+    )
+    file_meta = latest_version.changes.filter(
+        FileHistory.change == PushChangeType.UPDATE_DIFF.value
+    ).first()
+    assert file_meta.diff_file is not None
+    assert os.path.exists(
+        os.path.join(upload.project.storage.project_dir, file_meta.diff_file.location)
+    )
+
+    # above limit -> diff construction is skipped, plain force update
+    working_file = os.path.join(working_dir, "base.gpkg")
+    sql = "INSERT INTO simple (geometry, name) VALUES (GeomFromText('POINT(24.5, 38.2)', 4326), 'insert_test')"
+    execute_query(working_file, sql)
+    updated_gpkg_size = os.path.getsize(working_file)
+    changes["updated"] = [
+        file_info(working_dir, "base.gpkg", chunk_size=CHUNK_SIZE),
+        file_info(working_dir, "test.txt", chunk_size=CHUNK_SIZE),
+    ]
+    upload, upload_dir = create_transaction("mergin", changes, version=2)
+    upload_chunks(upload_dir, upload.changes, src_dir=working_dir)
+    with patch.dict(
+        client.application.config,
+        {"MAX_DIFFABLE_FORCE_UPDATE_SIZE": updated_gpkg_size - 1},
+    ):
+        resp = client.post(f"/v1/project/push/finish/{upload.transaction_id}")
+    assert resp.status_code == 200
+    latest_version = upload.project.get_latest_version()
+    assert (
+        latest_version.changes.filter(
+            FileHistory.change == PushChangeType.UPDATE.value
+        ).count()
+        == 2
+    )
+    assert not latest_version.changes.filter(
+        FileHistory.change == PushChangeType.UPDATE_DIFF.value
+    ).count()
+    assert all(
+        file_meta.diff_file is None
+        for file_meta in latest_version.changes.filter(
+            FileHistory.change == PushChangeType.UPDATE.value
+        ).all()
+    )
+    version_files = os.listdir(
+        os.path.join(upload.project.storage.project_dir, f"v{latest_version.name}")
+    )
+    diff_files = [f for f in version_files if re.findall("-diff-", f)]
+    assert not diff_files
+
+
 clone_project_data = [
     ({"project": " clone "}, "mergin", 200),  # clone own project
     (
@@ -2629,6 +2725,43 @@ def test_filepath_manipulation(client):
     )
 
 
+def test_diff_filepath_manipulation(client):
+    """Test path validation of the nested diff file during file upload"""
+    push_start_url = url_for(
+        f"/v1.mergin_sync_public_api_controller_project_push",
+        namespace=test_workspace_name,
+        project_name=test_project,
+    )
+    filename = "data.gpkg"
+    with open(os.path.join(TMP_DIR, filename), "w") as f:
+        f.write("Hello, Mergin!")
+    changes = {
+        "added": [],
+        "updated": [file_info(TMP_DIR, filename, chunk_size=CHUNK_SIZE)],
+        "removed": [],
+    }
+    # Manipulate the diff's path by prepending ../../
+    manipulated_diff_path = "../../" + filename
+    changes["updated"][0]["diff"] = {
+        "path": manipulated_diff_path,
+        "checksum": changes["updated"][0]["checksum"],
+        "size": changes["updated"][0]["size"],
+    }
+    # Block upload in push_start because of the invalid diff path
+    resp = client.post(
+        push_start_url,
+        data=json.dumps(
+            {"version": "v1", "changes": changes}, cls=DateTimeEncoder
+        ).encode("utf-8"),
+        headers=json_headers,
+    )
+    assert resp.status_code == 400
+    assert (
+        resp.json["detail"]
+        == f"Unsupported file name detected: '{manipulated_diff_path}'. Please remove the invalid characters."
+    )
+
+
 def test_supported_file_upload(client):
     """Test rejecting unsupported file based on extension and its mime type"""
     push_start_url = url_for(