diff --git a/ctfcli/core/properties/scalars.py b/ctfcli/core/properties/scalars.py index 1a1c7d4..ad1a72f 100644 --- a/ctfcli/core/properties/scalars.py +++ b/ctfcli/core/properties/scalars.py @@ -137,14 +137,46 @@ class AttemptsProperty(Property): def is_default(self, value) -> bool: return value == 0 + @classmethod + def parse(cls, value: Any, challenge_file_path=None) -> int: + # CTFd uses 0 for unlimited attempts, and rejects null or negative values + error = ( + f"Challenge file at {challenge_file_path} has an invalid 'attempts' value '{value}': " + "expected a non-negative integer, use 0 for unlimited attempts" + ) + + # if attempts is an int as string, cast it + if isinstance(value, str) and value.lstrip("-").isdigit(): + value = int(value) + + # bool is a subclass of int, but attempts: true is not a valid value + if not isinstance(value, int) or isinstance(value, bool) or value < 0: + raise InvalidChallengeFile(error) + + return value + def create_payload(self, ctx: PropertyContext) -> dict: if self.ignored(ctx): return {} - return {"max_attempts": ctx.challenge.get("attempts", 0)} + # parse validates the attempts value and raises otherwise + attempts = self.parse(ctx.challenge.get("attempts", 0), ctx.challenge.challenge_file_path) + + return {"max_attempts": attempts} def pull(self, ctx: PropertyContext, remote_data: dict): - return remote_data["max_attempts"] + # Older CTFd versions accepted a null max_attempts, which behaves as unlimited + return remote_data["max_attempts"] or 0 + + # Check that attempts, if present, is a non-negative integer + def lint(self, challenge, issues: dict) -> None: + if "attempts" not in challenge: + return + + try: + self.parse(challenge["attempts"], challenge.challenge_file_path) + except InvalidChallengeFile as e: + issues["fields"].append(str(e)) class ConnectionInfoProperty(CopiedProperty): diff --git a/ctfcli/spec/challenge-example.yml b/ctfcli/spec/challenge-example.yml index 9b8f46d..29f30bd 100644 --- a/ctfcli/spec/challenge-example.yml +++ b/ctfcli/spec/challenge-example.yml @@ -65,7 +65,8 @@ healthcheck: writeup/exploit.sh # state: solved # hidden | visible | solved solution: writeup/WRITEUP.md -# Can be removed if unused +# Maximum amount of attempts, must be a non-negative integer +# Can be removed if unused, which defaults to 0 (unlimited attempts) attempts: 5 # Specifies flag processing behavior of challenge diff --git a/tests/core/test_challenge.py b/tests/core/test_challenge.py index a0bd7be..1c3cc46 100644 --- a/tests/core/test_challenge.py +++ b/tests/core/test_challenge.py @@ -2977,3 +2977,89 @@ def test_lint_accepts_timezone_aware_scheduled_at(self): challenge = Challenge(self.minimal_challenge, {"scheduled_at": "2026-06-15T12:00:00+00:00"}) # Should not raise for scheduled_at (no other lint issues in the minimal fixture) self.assertTrue(challenge.lint(skip_hadolint=True)) + + +class TestChallengeAttempts(unittest.TestCase): + minimal_challenge = BASE_DIR / "fixtures" / "challenges" / "test-challenge-minimal" / "challenge.yml" + + def _prop(self): + return get_property("attempts") + + def _payload(self, challenge, ignore=()): + return self._prop().create_payload(PropertyContext(challenge, ignore=ignore)) + + def test_parse_accepts_non_negative_integers(self): + self.assertEqual(self._prop().parse(0), 0) + self.assertEqual(self._prop().parse(5), 5) + + def test_parse_casts_integer_strings(self): + self.assertEqual(self._prop().parse("5"), 5) + + def test_parse_rejects_null(self): + # CTFd uses 0 for unlimited attempts, a null max_attempts breaks the challenge view + with self.assertRaises(InvalidChallengeFile) as ctx: + self._prop().parse(None) + self.assertIn("0 for unlimited", str(ctx.exception)) + + def test_parse_rejects_negative_values(self): + with self.assertRaises(InvalidChallengeFile): + self._prop().parse(-1) + + with self.assertRaises(InvalidChallengeFile): + self._prop().parse("-1") + + def test_parse_rejects_non_integer_values(self): + for value in [True, 1.5, "five", ""]: + with self.subTest(value=value), self.assertRaises(InvalidChallengeFile): + self._prop().parse(value) + + def test_payload_defaults_to_unlimited_when_absent(self): + challenge = Challenge(self.minimal_challenge) + self.assertEqual(self._payload(challenge), {"max_attempts": 0}) + + def test_payload_includes_attempts(self): + challenge = Challenge(self.minimal_challenge, {"attempts": 5}) + self.assertEqual(self._payload(challenge), {"max_attempts": 5}) + + def test_payload_raises_on_null_attempts(self): + challenge = Challenge(self.minimal_challenge, {"attempts": None}) + with self.assertRaises(InvalidChallengeFile): + self._payload(challenge) + + def test_payload_raises_on_negative_attempts(self): + challenge = Challenge(self.minimal_challenge, {"attempts": -1}) + with self.assertRaises(InvalidChallengeFile): + self._payload(challenge) + + def test_payload_omits_attempts_when_ignored(self): + challenge = Challenge(self.minimal_challenge, {"attempts": None}) + self.assertEqual(self._payload(challenge, ignore=("attempts",)), {}) + + @mock.patch("ctfcli.core.challenge.API") + def test_create_does_not_post_null_attempts(self, mock_api_constructor: MagicMock): + challenge = Challenge(self.minimal_challenge, {"attempts": None}) + with self.assertRaises(InvalidChallengeFile): + challenge.create() + mock_api_constructor.return_value.post.assert_not_called() + + def test_pull_normalizes_null_remote_attempts(self): + # Older CTFd versions accepted a null max_attempts, which means unlimited + ctx = PropertyContext(Challenge(self.minimal_challenge)) + self.assertEqual(self._prop().pull(ctx, {"max_attempts": None}), 0) + self.assertEqual(self._prop().pull(ctx, {"max_attempts": 5}), 5) + + def test_lint_flags_null_attempts(self): + challenge = Challenge(self.minimal_challenge, {"attempts": None}) + with self.assertRaises(LintException) as ctx: + challenge.lint(skip_hadolint=True) + self.assertIn("attempts", " ".join(ctx.exception.issues["fields"])) + + def test_lint_flags_negative_attempts(self): + challenge = Challenge(self.minimal_challenge, {"attempts": -1}) + with self.assertRaises(LintException) as ctx: + challenge.lint(skip_hadolint=True) + self.assertIn("attempts", " ".join(ctx.exception.issues["fields"])) + + def test_lint_accepts_valid_attempts(self): + challenge = Challenge(self.minimal_challenge, {"attempts": 0}) + self.assertTrue(challenge.lint(skip_hadolint=True))