fix dangling solution files#207
Conversation
|
Let's wait for CTFd/CTFd#3057 first and then this can merge. Unsure if they need to be related. |
|
This is still needed even with cascade deletes brought by CTFd/CTFd#3057. Cascading only removes the duplicates when solutions are removed - but without this PR ctfcli still accumulates solution files on sync. |
There was a problem hiding this comment.
Pull request overview
This PR updates the solution-sync flow so that previously uploaded solution files don’t accumulate as dangling artifacts when a solution is re-synced, and avoids orphaning files when the same image is referenced repeatedly in a writeup.
Changes:
- Delete previously referenced remote solution files before rebuilding and re-uploading solution content.
- Deduplicate identical markdown image references to avoid uploading the same image multiple times in a single sync.
- Add test coverage and fixtures for deletion behavior and repeated-image handling.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
ctfcli/core/challenge.py |
Adds solution-file cleanup based on previous solution content and deduplicates repeated markdown image references before upload. |
tests/core/test_challenge.py |
Extends tests to validate solution-file deletion, deduped deletions, and single upload for repeated references. |
tests/fixtures/challenges/test-challenge-solution/writeup/WRITEUP-DUPLICATE.md |
Adds a fixture writeup containing repeated image references used by the new tests. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| 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} |
| 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} | ||
|
|
||
| # 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}") | ||
| r.raise_for_status() | ||
|
|
There was a problem hiding this comment.
Why doesnt this delete the files that are connected by solution id? Searching by location could result in other non-related files being deleted no?
ctfcli always re-uploads the solution files without removing existing ones. This leads to leftover files accumulating with each sync . This PR adds removing of currently uploaded solution files before that re-upload (which is needed).