From 2d4596cffc6a07118d3b48371826a2a8e013d86d Mon Sep 17 00:00:00 2001
From: Mathieu Leplatre <mathieu@mozilla.com>
Date: Wed, 20 May 2026 15:43:33 +0200
Subject: [PATCH] Fix #1317: fix crash when attachment is deleted

---
 .../src/kinto_remote_settings/signer/utils.py | 22 +++++++-----
 .../tests/signer/test_utils.py                | 36 ++++++++++++++-----
 uv.lock                                       |  2 +-
 3 files changed, 42 insertions(+), 18 deletions(-)

diff --git a/kinto-remote-settings/src/kinto_remote_settings/signer/utils.py b/kinto-remote-settings/src/kinto_remote_settings/signer/utils.py
index 88e7c05c9..a90b071c9 100644
--- a/kinto-remote-settings/src/kinto_remote_settings/signer/utils.py
+++ b/kinto-remote-settings/src/kinto_remote_settings/signer/utils.py
@@ -286,19 +286,25 @@ def records_diff(left, right):
 
 def attachments_size_diff(left, right):
     """
-    Return the size of the new attachments in `left` list of records.
+    Return the net size of the attachments in `left` versus `right`.
     """
-    right_by_location = {
-        r["attachment"]["location"]: r["attachment"]["size"]
-        for r in right
-        if "attachment" in r
+    right_by_id = {r["id"]: r for r in right}
+    right_locations = {
+        r["attachment"]["location"] for r in right if r.get("attachment")
     }
     changed = 0
     for r in left:
-        if "attachment" not in r or r["attachment"]["location"] in right_by_location:
-            # Locations are unique. If present in both, no change.
+        if "attachment" not in r:
             continue
-        changed += r["attachment"]["size"]
+        attachment = r["attachment"]
+        if attachment is None:
+            # Attachment removed: subtract previous size if it existed.
+            previous = right_by_id.get(r["id"])
+            if previous and previous.get("attachment"):
+                changed -= previous["attachment"]["size"]
+        elif attachment["location"] not in right_locations:
+            # New file on storage (locations are unique).
+            changed += attachment["size"]
     return changed
 
 
diff --git a/kinto-remote-settings/tests/signer/test_utils.py b/kinto-remote-settings/tests/signer/test_utils.py
index e7c806e2d..723d3a693 100644
--- a/kinto-remote-settings/tests/signer/test_utils.py
+++ b/kinto-remote-settings/tests/signer/test_utils.py
@@ -308,6 +308,24 @@ def test_fetch_cert():
         ([], [], 0),
         # No attachments
         ([{"id": "a"}, {"id": "b"}], [{"id": "c"}], 0),
+        # Deleted attachment
+        (
+            [{"id": "1", "attachment": None}],
+            [{"id": "1", "attachment": {"location": "file1", "size": 100}}],
+            -100,
+        ),
+        # Previously deleted attachment
+        (
+            [{"id": "1", "attachment": {"location": "file1", "size": 100}}],
+            [{"id": "1", "attachment": None}],
+            100,
+        ),
+        # Created and deleted attachment
+        (
+            [{"id": "1", "attachment": None}],
+            [],
+            0,
+        ),
         # No change
         (
             [
@@ -321,37 +339,37 @@ def test_fetch_cert():
         # New file
         (
             [
-                {"attachment": {"location": "file1", "size": 100}},
-                {"attachment": {"location": "file2", "size": 200}},
+                {"id": "1", "attachment": {"location": "file1", "size": 100}},
+                {"id": "2", "attachment": {"location": "file2", "size": 200}},
             ],
             [
-                {"attachment": {"location": "file1", "size": 100}},
+                {"id": "1", "attachment": {"location": "file1", "size": 100}},
             ],
             200,
         ),
         # 2 new attachments
         (
             [
-                {"attachment": {"location": "a", "size": 50}},
-                {"attachment": {"location": "b", "size": 75}},
-                {"attachment": {"location": "c", "size": 25}},
+                {"id": "1", "attachment": {"location": "a", "size": 50}},
+                {"id": "2", "attachment": {"location": "b", "size": 75}},
+                {"id": "3", "attachment": {"location": "c", "size": 25}},
             ],
             [
-                {"attachment": {"location": "a", "size": 50}},
+                {"id": "1", "attachment": {"location": "a", "size": 50}},
             ],
             100,  # b + c
         ),
         (
             [
                 {"id": "1"},
-                {"attachment": {"location": "x", "size": 30}},
+                {"id": "2", "attachment": {"location": "x", "size": 30}},
             ],
             [],
             30,
         ),
         (
             [],
-            [{"attachment": {"location": "a", "size": 1}}],
+            [{"id": "1", "attachment": {"location": "a", "size": 1}}],
             0,
         ),
     ],
diff --git a/uv.lock b/uv.lock
index 4ecf68f44..9dd366264 100644
--- a/uv.lock
+++ b/uv.lock
@@ -466,7 +466,7 @@ requires-dist = [
     { name = "pygit2", specifier = ">=1.19.0" },
     { name = "pyjwt", specifier = ">=2.10.1" },
     { name = "python-decouple", specifier = ">=3.8" },
-    { name = "requests", specifier = ">=2.34.2" },
+    { name = "requests", specifier = ">=2.32.4" },
     { name = "sentry-sdk", specifier = ">=2.20.0" },
 ]