From 0ad459201a1c0acb1a16a47917d2712f491b49cc Mon Sep 17 00:00:00 2001 From: AlexNg Date: Wed, 25 Dec 2024 18:20:01 +0800 Subject: [PATCH 1/4] fix: None not defaulted to [] Looks like .get() does not set challenge_files to the default empty list when it is None, causing ctfcli to throw. A second potential error case happens when directly using dict["files"] without checking for a None value. Using the OR operator fixes this. Signed-off-by: AlexNg --- ctfcli/core/challenge.py | 25 ++++++++++++++----------- 1 file changed, 14 insertions(+), 11 deletions(-) diff --git a/ctfcli/core/challenge.py b/ctfcli/core/challenge.py index 8bb4c6e..68b8aed 100644 --- a/ctfcli/core/challenge.py +++ b/ctfcli/core/challenge.py @@ -252,7 +252,7 @@ def _load_challenge_id(self): def _validate_files(self): # if the challenge defines files, make sure they exist before making any changes to the challenge - for challenge_file in self.get("files", []): + for challenge_file in self.get("files") or []: if not (self.challenge_directory / challenge_file).exists(): raise InvalidChallengeFile(f"File {challenge_file} could not be loaded") @@ -364,7 +364,7 @@ def _create_file(self, local_path: Path): def _create_all_files(self): new_files = [] - for challenge_file in self["files"]: + for challenge_file in self.get("files") or []: new_files.append(("file", open(self.challenge_directory / challenge_file, mode="rb"))) files_payload = {"challenge_id": self.challenge_id, "type": "challenge"} @@ -588,8 +588,8 @@ def sync(self, ignore: Tuple[str] = ()) -> None: # Create / Upload files if "files" not in ignore: # Get basenames of local files to compare against remote files - local_files = {f.split("/")[-1]: f for f in self.get("files", [])} - remote_files = self._normalize_remote_files(remote_challenge.get("files", [])) + local_files = {f.split("/")[-1]: f for f in self.get("files") or []} + remote_files = self._normalize_remote_files(remote_challenge.get("files") or []) # Delete remote files which are no longer defined locally for remote_file in remote_files: @@ -761,7 +761,7 @@ def lint(self, skip_hadolint=False, flag_format="flag{") -> bool: click.secho("Skipping Hadolint", fg="yellow") # Check that all files exist - challenge_files = challenge.get("files", []) + challenge_files = challenge.get("files") or [] for challenge_file in challenge_files: challenge_file_path = self.challenge_directory / challenge_file @@ -794,9 +794,12 @@ def mirror(self, files_directory_name: str = "dist", ignore: Tuple[str] = ()) -> remote_challenge = self.load_installed_challenge(self.challenge_id) challenge = self._normalize_challenge(remote_challenge) + remote_challenge["files"] = remote_challenge.get("files") or [] + challenge["files"] = challenge.get("files") or [] + # Add files which are not handled in _normalize_challenge if "files" not in ignore: - local_files = {Path(f).name: f for f in challenge.get("files", [])} + local_files = {Path(f).name: f for f in challenge["files"]} # Update files for remote_file in remote_challenge["files"]: @@ -813,9 +816,6 @@ def mirror(self, files_directory_name: str = "dist", ignore: Tuple[str] = ()) -> challenge_files_directory.mkdir(parents=True, exist_ok=True) (challenge_files_directory / remote_file_name).write_bytes(r.content) - if "files" not in challenge: - challenge["files"] = [] - challenge["files"].append(f"{files_directory_name}/{remote_file_name}") # The file is already present in the challenge.yml - we know the desired path @@ -827,7 +827,7 @@ def mirror(self, files_directory_name: str = "dist", ignore: Tuple[str] = ()) -> # Soft-Delete files that are not present on the remote # Remove them from challenge.yml but do not delete them from disk remote_file_names = [f.split("/")[-1].split("?token=")[0] for f in remote_challenge["files"]] - challenge["files"] = [f for f in challenge.get("files", []) if Path(f).name in remote_file_names] + challenge["files"] = [f for f in challenge["files"] if Path(f).name in remote_file_names] for key in challenge.keys(): if key not in ignore: @@ -841,6 +841,9 @@ def verify(self, ignore: Tuple[str] = ()) -> bool: remote_challenge = self.load_installed_challenge(self.challenge_id) normalized_challenge = self._normalize_challenge(remote_challenge) + remote_challenge["files"] = remote_challenge.get("files") or [] + challenge["files"] = challenge.get("files") or [] + for key in normalized_challenge: if key in ignore: continue @@ -865,7 +868,7 @@ def verify(self, ignore: Tuple[str] = ()) -> bool: # Check if files defined in challenge.yml are present try: self._validate_files() - local_files = {Path(f).name: f for f in challenge.get("files", [])} + local_files = {Path(f).name: f for f in challenge["files"]} except InvalidChallengeFile: return False From 66eb8e1211e438b66e8c56c284697fded4c87563 Mon Sep 17 00:00:00 2001 From: AlexNg Date: Wed, 25 Dec 2024 18:20:53 +0800 Subject: [PATCH 2/4] fix: Remove unneeded .get() This removes the repeated .get(). We can use the challenge_files from the previous get() on line 764 as it remains unchanged. Signed-off-by: AlexNg --- ctfcli/core/challenge.py | 1 - 1 file changed, 1 deletion(-) diff --git a/ctfcli/core/challenge.py b/ctfcli/core/challenge.py index 68b8aed..ac340a0 100644 --- a/ctfcli/core/challenge.py +++ b/ctfcli/core/challenge.py @@ -771,7 +771,6 @@ def lint(self, skip_hadolint=False, flag_format="flag{") -> bool: ) # Check that files don't have a flag in them - challenge_files = challenge.get("files", []) for challenge_file in challenge_files: challenge_file_path = self.challenge_directory / challenge_file From 3981df20ed93970d35948712dfddb7fb33c45678 Mon Sep 17 00:00:00 2001 From: AlexNg Date: Fri, 27 Dec 2024 07:17:32 +0800 Subject: [PATCH 3/4] refactor: challenge_files -> files Signed-off-by: AlexNg --- ctfcli/core/challenge.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/ctfcli/core/challenge.py b/ctfcli/core/challenge.py index ac340a0..fb61b2d 100644 --- a/ctfcli/core/challenge.py +++ b/ctfcli/core/challenge.py @@ -761,8 +761,8 @@ def lint(self, skip_hadolint=False, flag_format="flag{") -> bool: click.secho("Skipping Hadolint", fg="yellow") # Check that all files exist - challenge_files = challenge.get("files") or [] - for challenge_file in challenge_files: + files = self.get("files") or [] + for challenge_file in files: challenge_file_path = self.challenge_directory / challenge_file if challenge_file_path.is_file() is False: @@ -771,7 +771,7 @@ def lint(self, skip_hadolint=False, flag_format="flag{") -> bool: ) # Check that files don't have a flag in them - for challenge_file in challenge_files: + for challenge_file in files: challenge_file_path = self.challenge_directory / challenge_file if not challenge_file_path.exists(): From bb08c7b2d5e55a30ecdb19e0419ba76702b81cfe Mon Sep 17 00:00:00 2001 From: AlexNg Date: Mon, 30 Dec 2024 00:16:58 +0800 Subject: [PATCH 4/4] refactor: Move "or []" fixes to their own line --- ctfcli/core/challenge.py | 15 ++++++++++----- 1 file changed, 10 insertions(+), 5 deletions(-) diff --git a/ctfcli/core/challenge.py b/ctfcli/core/challenge.py index fb61b2d..553cc91 100644 --- a/ctfcli/core/challenge.py +++ b/ctfcli/core/challenge.py @@ -251,8 +251,8 @@ def _load_challenge_id(self): raise RemoteChallengeNotFound(f"Could not load remote challenge with name '{self['name']}'") def _validate_files(self): - # if the challenge defines files, make sure they exist before making any changes to the challenge - for challenge_file in self.get("files") or []: + files = self.get("files") or [] + for challenge_file in files: if not (self.challenge_directory / challenge_file).exists(): raise InvalidChallengeFile(f"File {challenge_file} could not be loaded") @@ -364,7 +364,9 @@ def _create_file(self, local_path: Path): def _create_all_files(self): new_files = [] - for challenge_file in self.get("files") or []: + + files = self.get("files") or [] + for challenge_file in files: new_files.append(("file", open(self.challenge_directory / challenge_file, mode="rb"))) files_payload = {"challenge_id": self.challenge_id, "type": "challenge"} @@ -587,9 +589,12 @@ def sync(self, ignore: Tuple[str] = ()) -> None: # Create / Upload files if "files" not in ignore: + self["files"] = self.get("files") or [] + remote_challenge["files"] = remote_challenge.get("files") or [] + # Get basenames of local files to compare against remote files - local_files = {f.split("/")[-1]: f for f in self.get("files") or []} - remote_files = self._normalize_remote_files(remote_challenge.get("files") or []) + local_files = {f.split("/")[-1]: f for f in self["files"]} + remote_files = self._normalize_remote_files(remote_challenge["files"]) # Delete remote files which are no longer defined locally for remote_file in remote_files: