From ed732561e74c578b73788a016f8ebe328257cfbd Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mi=C5=82osz=20Skaza?= Date: Mon, 1 Jun 2026 14:06:18 +0200 Subject: [PATCH 1/2] fix dangling solution files --- ctfcli/core/challenge.py | 19 +++++++++++ tests/core/test_challenge.py | 63 ++++++++++++++++++++++++++++++++++++ 2 files changed, 82 insertions(+) diff --git a/ctfcli/core/challenge.py b/ctfcli/core/challenge.py index d87f807..9249c50 100644 --- a/ctfcli/core/challenge.py +++ b/ctfcli/core/challenge.py @@ -550,6 +550,20 @@ def _get_existing_solution_id(self) -> int | None: return solution["id"] return None + def _delete_solution_files(self, content: str) -> None: + locations = re.findall(r"/files/([^)\s]+)", content or "") + if not locations: + return + + remote_files = self.api.get("/api/v1/files?type=solution").json()["data"] + file_ids_by_location = {f["location"]: f["id"] for f in remote_files} + + for location in locations: + file_id = file_ids_by_location.get(location) + if file_id is not None: + r = self.api.delete(f"/api/v1/files/{file_id}") + r.raise_for_status() + def _create_solution(self): resolved_solution = self._resolve_solution_path() if not resolved_solution: @@ -564,6 +578,11 @@ def _create_solution(self): r.raise_for_status() solution_id = r.json()["data"]["id"] else: + r = self.api.get(f"/api/v1/solutions/{solution_id}") + r.raise_for_status() + previous_content = r.json()["data"].get("content") or "" + self._delete_solution_files(previous_content) + # Keep solution state in sync and clear stale content before rebuilding references. r = self.api.patch( f"/api/v1/solutions/{solution_id}", diff --git a/tests/core/test_challenge.py b/tests/core/test_challenge.py index de892db..1a188f2 100644 --- a/tests/core/test_challenge.py +++ b/tests/core/test_challenge.py @@ -267,6 +267,25 @@ def mock_get(*args, **kwargs): "data": [{"id": 9, "challenge_id": 1, "state": "hidden", "content": "old"}], } return mock_response + if path == "/api/v1/solutions/9": + mock_response = MagicMock() + mock_response.json.return_value = { + "success": True, + "data": { + "id": 9, + "challenge_id": 1, + "state": "hidden", + "content": "old ![x](/files/stale-location/old.png)", + }, + } + return mock_response + if path == "/api/v1/files?type=solution": + mock_response = MagicMock() + mock_response.json.return_value = { + "success": True, + "data": [{"id": 42, "location": "stale-location/old.png"}], + } + return mock_response return MagicMock() mock_api: MagicMock = mock_api_constructor.return_value @@ -275,6 +294,8 @@ def mock_get(*args, **kwargs): challenge._create_solution() mock_api.post.assert_not_called() + # Previously uploaded solution files should be deleted before re-uploading + mock_api.delete.assert_called_once_with("/api/v1/files/42") mock_api.patch.assert_has_calls( [ call("/api/v1/solutions/9", json={"state": "solved", "content": ""}), @@ -283,6 +304,48 @@ def mock_get(*args, **kwargs): any_order=True, ) + @mock.patch("ctfcli.core.challenge.API") + def test_delete_solution_files_removes_referenced_files(self, mock_api_constructor: MagicMock): + challenge = Challenge(self.minimal_challenge) + challenge.challenge_id = 1 + + def mock_get(*args, **kwargs): + if args[0] == "/api/v1/files?type=solution": + mock_response = MagicMock() + mock_response.json.return_value = { + "success": True, + "data": [ + {"id": 1, "location": "loc-a/a.png"}, + {"id": 2, "location": "loc-b/b.png"}, + {"id": 3, "location": "loc-c/c.png"}, + ], + } + return mock_response + return MagicMock() + + mock_api: MagicMock = mock_api_constructor.return_value + mock_api.get.side_effect = mock_get + + challenge._delete_solution_files("![a](/files/loc-a/a.png) and ![c](/files/loc-c/c.png)") + + # Only files referenced in the content should be deleted, not the unreferenced one + mock_api.delete.assert_has_calls( + [call("/api/v1/files/1"), call("/api/v1/files/3")], + any_order=True, + ) + self.assertEqual(mock_api.delete.call_count, 2) + + @mock.patch("ctfcli.core.challenge.API") + def test_delete_solution_files_noop_without_references(self, mock_api_constructor: MagicMock): + challenge = Challenge(self.minimal_challenge) + challenge.challenge_id = 1 + + mock_api: MagicMock = mock_api_constructor.return_value + challenge._delete_solution_files("no files referenced here") + + mock_api.get.assert_not_called() + mock_api.delete.assert_not_called() + @mock.patch("ctfcli.core.challenge.API") def test_does_not_create_solution_if_not_specified(self, mock_api_constructor: MagicMock): challenge = Challenge(self.minimal_challenge) From a533efaa20dc1b112f500f62555407e6c344af2b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mi=C5=82osz=20Skaza?= Date: Fri, 17 Jul 2026 08:13:25 +0200 Subject: [PATCH 2/2] fix duplicate solutions erroring --- ctfcli/core/challenge.py | 8 ++- tests/core/test_challenge.py | 69 +++++++++++++++++++ .../writeup/WRITEUP-DUPLICATE.md | 7 ++ 3 files changed, 82 insertions(+), 2 deletions(-) create mode 100644 tests/fixtures/challenges/test-challenge-solution/writeup/WRITEUP-DUPLICATE.md diff --git a/ctfcli/core/challenge.py b/ctfcli/core/challenge.py index 9249c50..93a077e 100644 --- a/ctfcli/core/challenge.py +++ b/ctfcli/core/challenge.py @@ -558,7 +558,9 @@ def _delete_solution_files(self, content: str) -> None: remote_files = self.api.get("/api/v1/files?type=solution").json()["data"] file_ids_by_location = {f["location"]: f["id"] for f in remote_files} - for location in locations: + # A writeup can reference the same file more than once - deduplicate so + # that each file is only deleted once (a second DELETE would 404) + for location in dict.fromkeys(locations): file_id = file_ids_by_location.get(location) if file_id is not None: r = self.api.delete(f"/api/v1/files/{file_id}") @@ -596,7 +598,9 @@ def _create_solution(self): # Find all images in the content (markdown format; ignore html format) # Markdown format: ![alt text](image_url) # Returns tuples: (full_match, alt_text, image_path) - markdown_images = re.findall(r"(!\[([^\]]*)\]\(([^\)]+)\))", content) + # content.replace() below rewrites every occurrence, so uploading once per + # regex match would orphan a file for each repeated reference. Deduplicate. + markdown_images = list(dict.fromkeys(re.findall(r"(!\[([^\]]*)\]\(([^\)]+)\))", content))) # Find all snippet includes (MkDocs style: --8<-- "filename") # Returns tuples: (full_match, filename) diff --git a/tests/core/test_challenge.py b/tests/core/test_challenge.py index 1a188f2..cbd3220 100644 --- a/tests/core/test_challenge.py +++ b/tests/core/test_challenge.py @@ -335,6 +335,75 @@ def mock_get(*args, **kwargs): ) self.assertEqual(mock_api.delete.call_count, 2) + @mock.patch("ctfcli.core.challenge.API") + def test_delete_solution_files_deduplicates_repeated_references(self, mock_api_constructor: MagicMock): + challenge = Challenge(self.minimal_challenge) + challenge.challenge_id = 1 + + def mock_get(*args, **kwargs): + if args[0] == "/api/v1/files?type=solution": + mock_response = MagicMock() + mock_response.json.return_value = { + "success": True, + "data": [{"id": 7, "location": "loc-dup/dup.png"}], + } + return mock_response + return MagicMock() + + mock_api: MagicMock = mock_api_constructor.return_value + mock_api.get.side_effect = mock_get + + challenge._delete_solution_files("![a](/files/loc-dup/dup.png) and again ![a](/files/loc-dup/dup.png)") + + # A file referenced twice must only be deleted once - a second DELETE would 404 + mock_api.delete.assert_called_once_with("/api/v1/files/7") + + @mock.patch("ctfcli.core.challenge.API") + def test_creates_solution_uploads_repeated_image_only_once(self, mock_api_constructor: MagicMock): + challenge = Challenge( + self.solution_challenge, + {"solution": {"path": "writeup/WRITEUP-DUPLICATE.md", "state": "hidden"}}, + ) + challenge.challenge_id = 1 + + def mock_get(*args, **kwargs): + if args[0] == "/api/v1/solutions": + mock_response = MagicMock() + mock_response.json.return_value = {"success": True, "data": []} + return mock_response + return MagicMock() + + def mock_post(*args, **kwargs): + path = args[0] + if path == "/api/v1/solutions": + mock_response = MagicMock() + mock_response.json.return_value = {"success": True, "data": {"id": 5}} + return mock_response + if path == "/api/v1/files": + mock_response = MagicMock() + mock_response.json.return_value = { + "success": True, + "data": [{"location": "uploaded-location/test.png"}], + } + return mock_response + return MagicMock() + + mock_api: MagicMock = mock_api_constructor.return_value + mock_api.get.side_effect = mock_get + mock_api.post.side_effect = mock_post + + challenge._create_solution() + + # The image is referenced twice but must only be uploaded once - content.replace + # rewrites every occurrence, so a second upload would be orphaned immediately + file_uploads = [c for c in mock_api.post.call_args_list if c.args[0] == "/api/v1/files"] + self.assertEqual(len(file_uploads), 1) + + # Both references should still be rewritten to the uploaded location + patched_content = mock_api.patch.call_args_list[-1].kwargs["json"]["content"] + self.assertEqual(patched_content.count("/files/uploaded-location/test.png"), 2) + self.assertNotIn("](images/test.png)", patched_content) + @mock.patch("ctfcli.core.challenge.API") def test_delete_solution_files_noop_without_references(self, mock_api_constructor: MagicMock): challenge = Challenge(self.minimal_challenge) diff --git a/tests/fixtures/challenges/test-challenge-solution/writeup/WRITEUP-DUPLICATE.md b/tests/fixtures/challenges/test-challenge-solution/writeup/WRITEUP-DUPLICATE.md new file mode 100644 index 0000000..c01dada --- /dev/null +++ b/tests/fixtures/challenges/test-challenge-solution/writeup/WRITEUP-DUPLICATE.md @@ -0,0 +1,7 @@ +# Solution + +The same image is referenced more than once: +![diagram](images/test.png) + +And here it is again: +![diagram](images/test.png)