Repository navigation
feat(tooling): JDG-11 remainder + RO-6 closure — validator runner, output generation, package import, archive axis [Wave C] - #307
Conversation
Codex Review Summary: ✅ CompletedThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
![sentry[bot]](https://avatars.githubusercontent.com/in/12637?s=60&v=4){kind=small}
| node_id = self.request.query_params.get("curriculum_node") | ||
| if node_id: | ||
| qs = qs.filter(curriculum_nodes__id=node_id) | ||
| # Archive browse axis (competitive roadmap RO-6): the olympiad | ||
| # archive's provenance fields are a first-class catalog filter — | ||
| # browse by edition year, grade group, or "archive problems only". | ||
| source_year = self.request.query_params.get("source_year") | ||
| if source_year: | ||
| qs = qs.filter(source_year=source_year) |
There was a problem hiding this comment.
Bug: Unvalidated source_year query parameter passed directly to qs.filter() causes an unhandled ValueError when non-integer values are provided.
Severity: MEDIUM
Suggested Fix
Validate that source_year contains an integer value (for example, using a try/except ValueError block or checking source_year.isdigit()) before filtering, or handle invalid values by returning an HTTP 400 response or ignoring the filter.
Prompt for AI Agent
Review the code at the location below. A potential bug has been identified by an AI
agent. Verify if this is a real issue. If it is, propose a fix; if not, explain why it's
not valid.
Location: backend/problems/views.py#L260-L268
Potential issue: In `ProblemViewSet.get_queryset`, the `source_year` query parameter is
directly passed to `qs.filter(source_year=source_year)` without validation or casting.
Since `source_year` is an integer field (`PositiveSmallIntegerField`), non-integer
values such as decimals (`2024.5`), exponential notation (`1e3`), or non-numeric strings
cause Django's query engine to raise an unhandled `ValueError` when preparing the
lookup, resulting in an unhandled exception for requests against the problems list
endpoint.
Did we get this right? 👍 / 👎 to inform future reviews.
| import_source = serializers.PrimaryKeyRelatedField( | ||
| queryset=ImportSource.objects.all(), | ||
| required=False, | ||
| allow_null=True, | ||
| ) |
There was a problem hiding this comment.
Bug: import_source is declared as a writable field on problem updates, allowing authors to clear or swap it to bypass publication checks.
Severity: MEDIUM
Suggested Fix
Mark import_source as read-only on update operations in the problem serializer, or restrict modifications to authorized administrative roles.
Prompt for AI Agent
Review the code at the location below. A potential bug has been identified by an AI
agent. Verify if this is a real issue. If it is, propose a fix; if not, explain why it's
not valid.
Location: backend/problems/serializers.py#L94-L98
Potential issue: The `import_source` field on the problem serializer is defined as a
writable `PrimaryKeyRelatedField` and is not included in `read_only_fields`. Authors
editing problems in `DRAFT` status or users with edit permissions can send a `PATCH`
request to set `import_source` to `null` or change it to another record. This bypasses
the publication rights gate during submission and approval, which only checks whether
`import_source_id` is present and allowed.
Also affects:
backend/problems/views.py:288~306
Did we get this right? 👍 / 👎 to inform future reviews.
| whiteSpace: 'nowrap', | ||
| }} | ||
| > | ||
| {[problem.source_contest, problem.source_year, problem.grade_group] | ||
| .filter(Boolean) | ||
| .join(' · ')} | ||
| </span> | ||
| )} | ||
| </td> | ||
| <td style={{ padding: '14px 20px' }}> | ||
| <span style={{ |
There was a problem hiding this comment.
Bug: loadMore pagination omits archive filter parameters when requesting subsequent pages, appending unfiltered problems into the filtered list view.
Severity: MEDIUM
Suggested Fix
Update ProblemService.getProblems in src/services/ApiService.ts to accept archive filter parameters, and pass the current active archive filters in the loadMore call in ProblemsListClient.tsx.
Prompt for AI Agent
Review the code at the location below. A potential bug has been identified by an AI
agent. Verify if this is a real issue. If it is, propose a fix; if not, explain why it's
not valid.
Location: src/app/problems/ProblemsListClient.tsx#L760-L786
Potential issue: In `ProblemsListClient`, subsequent pages loaded via `loadMore` call
`ProblemService.getProblems` without passing archive filter parameters (`archive`,
`sourceYear`, `gradeGroup`). Because the client-side continuation does not pass these
parameters and `ProblemService.getProblems` does not support them, pages beyond page 1
are fetched unfiltered. These unfiltered results are then appended directly into the
local problem list, displaying non-archive problems under the filtered archive view when
results exceed 100 items.
Also affects:
src/app/problems/_lib/problems-api.ts:43~68
Did we get this right? 👍 / 👎 to inform future reviews.
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 983732e7d0
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| ok, executable, compiler_output = _compile_or_400( | ||
| data[code_key], data[lang_key], workspace_dir, key, request | ||
| ) |
There was a problem hiding this comment.
When the generator and validator use the same language, the second compilation overwrites the first program: compile_code_in_sandbox writes to the registry's fixed source filename and artifact name within workspace_dir. For example, two Python programs both resolve to user_code.py, so the generator actually executes the validator. The generated-input branch of GenerateOutputsView has the same issue, with the generator overwriting the reference solution and silently producing incorrect expected outputs. Use separate program directories for compilation and execution; distinct executable variables do not isolate the underlying files.
Useful? React with 👍 / 👎.
| verdict, out, err = _run_full( | ||
| ref_exec, data["reference_language"], stdin_data, workspace_dir | ||
| ) | ||
| entry = {"input": stdin_data[:20000], "output": out[:20000]} |
There was a problem hiding this comment.
For any input or reference output longer than 20,000 characters, this returns a silently truncated test pair even when execution succeeds. The reference ran against the complete input, so truncating the returned input can also make its expected output inconsistent with it. Since these results are intended to become real test cases, this can cause correct submissions to fail against corrupted judge data. Return complete pairs, or explicitly reject oversized results rather than presenting them as successful output generation.
Useful? React with 👍 / 👎.
| except (TypeError, ValueError): | ||
| time_ms = 1000 | ||
| try: | ||
| memory_kb = int(limits.get("memory_limit_bytes", 262144)) // 1024 |
There was a problem hiding this comment.
When problem.yaml contains a limits block but omits memory_limit_bytes, such as a package specifying only time_seconds, the fallback becomes 262144 // 1024, or 256 KB, rather than the intended 262144 KB. This value is persisted as default_memory_limit_kb and consumed by the submission judge, so an otherwise valid imported problem receives an unusably small memory budget. Default to the equivalent byte value before dividing, or retain the existing KB default when the key is absent.
Useful? React with 👍 / 👎.
| try { | ||
| const [firstPage] = await Promise.all([ | ||
| getProblemsPage(1, { curriculumNode: curriculumParam }), | ||
| getProblemsPage(1, { curriculumNode: curriculumParam, ...archiveFilters }), |
There was a problem hiding this comment.
When an archive/year/group selection has more than 100 matches, this filters only the server-rendered first page. ProblemsListClient.loadMore still calls ProblemService.getProblems({}, { page: nextPageRef.current, pageSize: 100 }), without any archive filters, and the client does not filter appended rows by provenance. Consequently, clicking Load more mixes unrelated problems into the archive results and can skip matching problems because the page offsets refer to different result sets. Pass the same filters through the browser-side pagination path.
Useful? React with 👍 / 👎.
| names = package.namelist() | ||
| if len(names) > MAX_PACKAGE_ENTRIES: |
There was a problem hiding this comment.
For a highly compressed package uploaded by an administrator or verifier, the 15 MB check bounds only compressed bytes, while the entry-count check does not bound individual or total uncompressed sizes. The subsequent package.read calls fully decompress statements and test files into the API worker's memory, and _tests_from_zip collects all pairs before applying the test cap. Even an archive with just two entries can therefore expand to gigabytes and exhaust the worker. Check aggregate and per-member uncompressed sizes before reading, and enforce bounded reads.
Useful? React with 👍 / 👎.
| or the caller must explicitly declare own/licensed content. | ||
| """ | ||
|
|
||
| permission_classes = [permissions.IsAuthenticated] |
There was a problem hiding this comment.
When TWO_FACTOR_ENFORCE=True, an administrator or verifier without enrolled TOTP can still create problems and test cases through this endpoint. The equivalent hand-authored creation path in ProblemViewSet.get_permissions explicitly includes PrivilegedRoleRequires2FA, but this view checks only authentication and role before writing the imported content; the existing permission is not enforced globally. Add that permission here so package import does not bypass the configured content-authoring restriction.
Useful? React with 👍 / 👎.
| import_source = serializers.PrimaryKeyRelatedField( | ||
| queryset=ImportSource.objects.all(), | ||
| required=False, | ||
| allow_null=True, | ||
| ) |
There was a problem hiding this comment.
For a draft whose attached import source is REJECTED or EXPIRED, an authorized editor can now PATCH {"import_source": null} and then submit it for approval. perform_update saves this field without any rights validation, and both new publication checks skip problems with a null import_source_id, so clearing the link bypasses the restriction without even requiring the import endpoint's explicit own-content declaration. Preserve the covering ledger link for imported material, or require an explicit, authorized reclassification before allowing it to be removed.
Useful? React with 👍 / 👎.
| status=status.HTTP_400_BAD_REQUEST, | ||
| ) | ||
|
|
||
| metadata: dict = {} | ||
| if "problem.yaml" in names: | ||
| metadata = parse_flat_yaml( | ||
| package.read("problem.yaml").decode("utf-8", errors="replace") | ||
| ) | ||
|
|
||
| en_statement, ro_statement, tex_title = _statement_from_zip(package) | ||
| tests = _tests_from_zip(package) |
There was a problem hiding this comment.
Bug: Package import does not restrict individual entry or total uncompressed byte size, allowing zip expansion to consume excessive memory.
Severity: MEDIUM
Suggested Fix
Inspect ZipInfo.file_size before decompression to enforce a maximum individual file size and cumulative uncompressed size threshold, rejecting packages that exceed bounds.
Prompt for AI Agent
Review the code at the location below. A potential bug has been identified by an AI
agent. Verify if this is a real issue. If it is, propose a fix; if not, explain why it's
not valid.
Location: backend/problems/tooling.py#L885-L899
Potential issue: The package import logic checks the uploaded compressed size and total
entry count, but does not bound the uncompressed size of individual files or the archive
as a whole. `_statement_from_zip` and `_tests_from_zip` decompress and load matching
entries into memory via `package.read()` before `MAX_IMPORT_TESTS` is sliced, and the
test inputs/outputs are saved to `TestCase` without size limits. Highly compressed
archives can consume gigabytes of memory, risking worker process out-of-memory crashes.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 5144762e11
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| reference_code = serializers.CharField() | ||
| # Either explicit inputs (list of stdin blobs)… | ||
| inputs = serializers.ListField( | ||
| child=serializers.CharField(allow_blank=True), required=False, max_length=50 |
There was a problem hiding this comment.
For whitespace-sensitive problems, DRF's CharField defaults to trim_whitespace=True, so this serializer removes leading/trailing spaces and newlines before the reference solution runs. For example, an input ending with an empty line becomes a different test, and an all-whitespace input becomes empty. The endpoint consequently generates expected outputs for modified inputs rather than the supplied test cases. Set trim_whitespace=False on the child field so stdin blobs reach the reference unchanged.
Useful? React with 👍 / 👎.
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
| if isinstance(limits, dict): | ||
| try: | ||
| time_ms = int(float(limits.get("time_seconds", 1)) * 1000) | ||
| except (TypeError, ValueError): | ||
| time_ms = 1000 | ||
| try: | ||
| memory_kb = int(limits.get("memory_limit_bytes", 262144)) // 1024 | ||
| except (TypeError, ValueError): | ||
| memory_kb = 262144 |
There was a problem hiding this comment.
Bug: Parsing limits in ImportPackageView catches TypeError and ValueError, but fails to handle OverflowError or validate values exceeding standard database integer ranges.
Severity: LOW
Suggested Fix
Expand the exception handler to include OverflowError alongside TypeError and ValueError. Additionally, clamp or validate parsed time_ms and memory_kb within acceptable positive 32-bit integer limits before saving.
Prompt for AI Agent
Review the code at the location below. A potential bug has been identified by an AI
agent. Verify if this is a real issue. If it is, propose a fix; if not, explain why it's
not valid.
Location: backend/problems/tooling.py#L911-L919
Potential issue: In `ImportPackageView`, parsing `limits` from `metadata` uses
`int(float(limits.get("time_seconds", 1)) * 1000)` and integer conversions for
`memory_limit_bytes` inside `try...except (TypeError, ValueError)` blocks. Values
producing infinite floats like `inf` or `1e999` raise `OverflowError` during integer
conversion, which is not caught. Furthermore, values exceeding the 32-bit signed integer
boundary or negative values are not validated, triggering database `DataError`
exceptions upon inserting into `Problem.default_time_limit_ms` or
`default_memory_limit_kb` in PostgreSQL.
| created = 0 | ||
| for order, (_name, input_text, answer, is_sample) in enumerate(imported_tests): | ||
| TestCase.objects.create( | ||
| problem=problem, | ||
| input_data=input_text, | ||
| expected_output_data=answer, | ||
| is_sample=is_sample, | ||
| points=10, | ||
| order=order, | ||
| ) | ||
| created += 1 |
There was a problem hiding this comment.
Bug: Package import creates the problem and test cases non-atomically, potentially leaving orphaned partial problem data if an insert fails mid-loop.
Severity: LOW
Suggested Fix
Wrap the Problem.objects.create and the loop inserting TestCase instances in a transaction.atomic() block to ensure all-or-nothing execution.
Prompt for AI Agent
Review the code at the location below. A potential bug has been identified by an AI
agent. Verify if this is a real issue. If it is, propose a fix; if not, explain why it's
not valid.
Location: backend/problems/tooling.py#L921-L950
Potential issue: In `ImportPackageView.post`, creation of the `Problem` record and the
subsequent iteration creating individual `TestCase` rows are executed outside of a
database transaction. If an error occurs during test case creation—such as psycopg
rejecting a NUL byte or an intermittent database disruption—the `Problem` row and
previously inserted `TestCase` records remain committed, leaving an incomplete DRAFT
problem in the database.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: b30051fc72
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| source_year = self.request.query_params.get("source_year") | ||
| if source_year: | ||
| qs = qs.filter(source_year=source_year) |
There was a problem hiding this comment.
Entering a fractional year such as 2024.5 in the new archive year input causes its unconditional blur handler to put that value in source_year. Passing it directly to the model's integer-field filter raises an uncaught ValueError, so the API returns 500 and the catalog displays a loading error instead of results. The number input's min/max attributes do not prevent this navigation. Validate the query parameter before filtering and return a controlled validation response or ignore invalid values.
Useful? React with 👍 / 👎.
| if "problem.yaml" in names: | ||
| metadata = parse_flat_yaml( | ||
| package.read("problem.yaml").decode("utf-8", errors="replace") | ||
| ) | ||
|
|
||
| en_statement, ro_statement, tex_title = _statement_from_zip(package) | ||
| tests = _tests_from_zip(package) |
There was a problem hiding this comment.
Bug: Archive reads in ImportPackageView.post are not wrapped in exception handling, causing unhandled errors on corrupted, encrypted, or invalid zip members.
Severity: LOW
Suggested Fix
Wrap zip reading and extraction operations in a try/except (zipfile.BadZipFile, RuntimeError, NotImplementedError, zlib.error) block and return a 400 Bad Request error response.
Prompt for AI Agent
Review the code at the location below. A potential bug has been identified by an AI
agent. Verify if this is a real issue. If it is, propose a fix; if not, explain why it's
not valid.
Location: backend/problems/tooling.py#L893-L899
Potential issue: In `ImportPackageView.post`, only the initial base64 decoding and
opening of the zip archive via `zipfile.ZipFile` are wrapped in a `try/except` block
catching `BadZipFile`. Subsequent calls to `package.read` (such as reading
`problem.yaml`, problem statements, and test cases) are unhandled. If an archive has a
corrupted entry with a CRC mismatch, an encrypted member, or an unsupported compression
method, `ZipFile.read` can raise exceptions like `BadZipFile`, `RuntimeError`, or
`zlib.error`, causing an unhandled server error.
| def get_import_source_name(self, obj) -> str | None: | ||
| # Dotted sources raise on null FKs; the method form renders the | ||
| # ledger row's name or None without traversal gymnastics. | ||
| return obj.import_source.name if obj.import_source_id else None |
There was a problem hiding this comment.
Bug: Accessing obj.import_source.name in ProblemSerializer causes an N+1 query issue for imported problems because the queryset does not use select_related("import_source").
Severity: LOW
Suggested Fix
Add select_related("import_source") to ProblemViewSet.get_queryset in backend/problems/views.py when preparing the queryset for serialization.
Prompt for AI Agent
Review the code at the location below. A potential bug has been identified by an AI
agent. Verify if this is a real issue. If it is, propose a fix; if not, explain why it's
not valid.
Location: backend/problems/serializers.py#L100-L103
Potential issue: In `ProblemSerializer.get_import_source_name`, accessing
`obj.import_source.name` lazily loads the foreign key relation for any problem where
`import_source_id` is set. Because `ProblemViewSet.get_queryset` does not include
`select_related("import_source")`, listing problems results in an N+1 query pattern
where each imported problem in a page triggers an additional database query.
| words = raw_line.split(" ") | ||
| current = "" | ||
| for word in words: | ||
| candidate = f"{current} {word}".strip() | ||
| if len(candidate) > width and current: | ||
| lines.append(current) | ||
| current = word | ||
| else: | ||
| current = candidate | ||
| lines.append(current) | ||
| return lines or [""] |
There was a problem hiding this comment.
Bug: Lines exceeding 88 characters without spaces overflow page margins in generated PDFs, and long lines lose consecutive or leading whitespace.
Severity: LOW
Suggested Fix
Add a hard-break fallback to chop words exceeding width into segments of at most width characters. Preserve whitespace when splitting and reassembling lines rather than calling .strip() on long lines.
Prompt for AI Agent
Review the code at the location below. A potential bug has been identified by an AI
agent. Verify if this is a real issue. If it is, propose a fix; if not, explain why it's
not valid.
Location: backend/problems/pdf_bundle.py#L36-L50
Potential issue: In `_wrap`, lines longer than 88 characters are split only on space
characters without a hard-break fallback. An unbroken word longer than 88 characters
(such as continuous digit sequences or grid strings in competitive programming test
cases) bypasses length checks and is written as a single line, causing text to overflow
the page boundaries in generated PDFs. Additionally, splitting on spaces and joining via
`.strip()` collapses leading and consecutive whitespace, altering alignment for indented
sample lines exceeding 88 characters.
bc20edb to
549c7d7
Compare
| import_source = serializers.PrimaryKeyRelatedField( | ||
| queryset=ImportSource.objects.all(), | ||
| required=False, | ||
| allow_null=True, | ||
| ) |
There was a problem hiding this comment.
Bug: import_source is writable by authors on ProblemSerializer, allowing them to clear or forge the field and bypass publication rights checks.
Severity: MEDIUM
Suggested Fix
Add import_source to read_only_fields in ProblemSerializer, or restrict updates to administrative workflows rather than allowing standard author PATCH requests.
Prompt for AI Agent
Review the code at the location below. A potential bug has been identified by an AI
agent. Verify if this is a real issue. If it is, propose a fix; if not, explain why it's
not valid.
Location: backend/problems/serializers.py#L93-L97
Potential issue: `import_source` in `ProblemSerializer` is declared as a writable field
and is not included in `read_only_fields`. Authors editing a draft or private problem
can clear or alter `import_source` via PATCH requests. Since publication checks in
`submit_for_approval` and `approve_problem` only validate rights when `import_source_id`
is present, clearing the field bypasses publication authorization gates for revoked
sources or misattributes the source.
Also affects:
backend/problems/views.py:295~304backend/problems/views.py:325~334
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 549c7d72da
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| parts = re.split(r"[/\\]", name) | ||
| if len(parts) < 2 or parts[0].lower() != "statements": | ||
| continue | ||
| body = package.read(name).decode("utf-8", errors="replace") | ||
| lang = parts[1].lower() |
There was a problem hiding this comment.
When importing a Kattis package containing problem_statement/problem.en.tex, this condition skips the supplied statement because it accepts only statements/; the subsequent language detection also expects a language directory rather than a language-bearing filename. The Kattis specification defines the former layout. Executing this helper with that standard path returns empty statements, so an otherwise successful import replaces the complete English statement with “Imported package — statement pending.” Support the standard directory and filename-based language detection so authors do not have to manually restore statements already present in their packages.
Useful? React with 👍 / 👎.
| except (TypeError, ValueError): | ||
| memory_kb = 262144 | ||
|
|
||
| problem = Problem.objects.create( |
There was a problem hiding this comment.
For a package declaring validation: custom and supplying an output validator, the importer accepts its test pairs but discards the validation configuration and checker source. The Kattis specification explicitly distinguishes this mode from default comparison. The created problem instead retains LINES_STRIP_EXACT with no checker, so backend/submissions/tasks.py:565–599 compares submissions directly against the answer file; after approval, valid alternative answers are marked wrong. Either import the checker with the appropriate judging configuration or reject unsupported validation modes explicitly instead of returning a successful, silently altered problem.
Useful? React with 👍 / 👎.
…tput generation, package import, archive axis [Wave C] Content identical to the reviewed branch tree (7 commits, squashed here) — re-committed through the git data API so every change carries the linked-account attribution the base-branch policy requires (same remedy as #237→#238). See PR #307 for the full description and the locally-run gate battery: backend pytest 1754 passed / 100% line+branch / zero warnings; vitest 653; tsc clean; eslint 0/0; next build; playwright 131+3 skipped.
549c7d7 to
aadc776
Compare
| // so the client only needs the current value (not local state). | ||
| const urlCurriculum = searchParams.get('curriculum') ?? ''; | ||
| // [RO-6] The archive browse axis is server-filtered the same way: | ||
| // ?archive=1 + optional ?source_year= / ?grade_group= refinements. | ||
| const urlArchive = searchParams.get('archive') === '1'; | ||
| const urlYear = searchParams.get('source_year') ?? ''; | ||
| const urlGroup = searchParams.get('grade_group') ?? ''; | ||
| const [yearDraft, setYearDraft] = useState(urlYear); | ||
|
|
||
| const [search, setSearch] = useState(urlSearch); | ||
| const [difficultyFilter, setDifficultyFilter] = useState(urlDifficulty); |
There was a problem hiding this comment.
Bug: ProblemsListClient only recognizes archive=1 while the server and API accept archive=true, causing the toggle UI state to desync when accessed via archive=true.
Severity: LOW
Suggested Fix
Update urlArchive in ProblemsListClient.tsx to check for both '1' and 'true' (e.g., const archiveVal = searchParams.get('archive'); const urlArchive = archiveVal === '1' || archiveVal === 'true';).
Prompt for AI Agent
Review the code at the location below. A potential bug has been identified by an AI
agent. Verify if this is a real issue. If it is, propose a fix; if not, explain why it's
not valid.
Location: src/app/problems/ProblemsListClient.tsx#L179-L190
Potential issue: In `src/app/problems/page.tsx` and the backend, `archive=true` is
accepted as an alias to enable archive filtering on the server. However,
`ProblemsListClient` strictly checks `searchParams.get('archive') === '1'`. When
accessing the page with `archive=true`, the server returns archive-filtered problems,
but the client does not recognize the filter as active: the toggle button displays an
unpressed state and the secondary filter row remains hidden, causing an inconsistency
between the filtered results and UI state.
| setOrDelete( | ||
| 'source_year', | ||
| next.sourceYear !== undefined ? next.sourceYear : urlYear || undefined, | ||
| ); | ||
| setOrDelete( | ||
| 'grade_group', | ||
| next.gradeGroup !== undefined ? next.gradeGroup : urlGroup || undefined, | ||
| ); | ||
| const qs = params.toString(); | ||
| router.replace(qs ? `/problems?${qs}` : '/problems', { scroll: false }); | ||
| }; |
There was a problem hiding this comment.
Bug: loadMore does not pass archive filter parameters to ProblemService.getProblems, causing pages 2+ to return unfiltered problems when paginating.
Severity: MEDIUM
Suggested Fix
Extend ProblemService.getProblems (or reuse getProblemsPage) to accept and forward archive, source_year, and grade_group query parameters, and pass the active archive filter state into the loadMore request call.
Prompt for AI Agent
Review the code at the location below. A potential bug has been identified by an AI
agent. Verify if this is a real issue. If it is, propose a fix; if not, explain why it's
not valid.
Location: src/app/problems/ProblemsListClient.tsx#L232-L251
Potential issue: When loading additional problems via `loadMore`,
`ProblemService.getProblems` is called with an empty filter object `{}`. Furthermore,
`ProblemService.getProblems` does not accept `archive`, `source_year`, or `grade_group`
parameters. When a filtered archive list contains more than 100 problems and a user
clicks 'Load more problems', page 2+ requests are fetched without archive filters,
appending unrelated non-archive problems to the filtered list.
…rr/compiler output now answers from the persisted StressRun row (CodeQL py/stack-trace-exposure, 4 medium alerts)
3237e57 to
7d032d8
Compare
| data[code_key], data[lang_key], workspace_dir, key, request | ||
| ) | ||
| if not ok: | ||
| return Response( | ||
| persisted_detail, status=status.HTTP_400_BAD_REQUEST | ||
| ) | ||
| if key == "generator": | ||
| gen_exec, gen_lang = executable, data[lang_key] | ||
| else: | ||
| val_exec, val_lang = executable, data[lang_key] | ||
|
|
There was a problem hiding this comment.
Bug: Same-language programs compiled into the shared workspace directory overwrite each other's source files or artifacts, causing the wrong program to execute.
Severity: HIGH
Suggested Fix
Compile and run each program in its own distinct subdirectory within the workspace or assign unique source and artifact filenames per program.
Prompt for AI Agent
Review the code at the location below. A potential bug has been identified by an AI
agent. Verify if this is a real issue. If it is, propose a fix; if not, explain why it's
not valid.
Location: backend/problems/tooling.py#L490-L505
Potential issue: In `ValidatorRunView` and `GenerateOutputsView`, compiling multiple
programs of the same language (such as a generator and validator, or generator and
reference solution) writes to the same `workspace_dir`. Languages in `LANGUAGES` map to
fixed source filenames (e.g., `user_code.py`) and compile artifact names. When both
programs share the same language, the second compilation overwrites the first.
Consequently, running what was intended to be the generator executes the second program
instead, leading to runtime errors or incorrect output generation.
Also affects:
backend/problems/tooling.py:613~646
| status=status.HTTP_400_BAD_REQUEST, | ||
| ) | ||
|
|
||
| metadata: dict = {} | ||
| if "problem.yaml" in names: | ||
| metadata = parse_flat_yaml( | ||
| package.read("problem.yaml").decode("utf-8", errors="replace") | ||
| ) | ||
|
|
||
| en_statement, ro_statement, tex_title = _statement_from_zip(package) | ||
| tests = _tests_from_zip(package) |
There was a problem hiding this comment.
Bug: ImportPackageView decompresses zip archive contents into memory without checking uncompressed member file sizes, enabling memory exhaustion via zip bomb payloads.
Severity: MEDIUM
Suggested Fix
Check ZipInfo.file_size against per-file and total uncompressed size limits before calling read(), and break test pair collection early once MAX_IMPORT_TESTS is reached.
Prompt for AI Agent
Review the code at the location below. A potential bug has been identified by an AI
agent. Verify if this is a real issue. If it is, propose a fix; if not, explain why it's
not valid.
Location: backend/problems/tooling.py#L889-L910
Potential issue: `ImportPackageView` unpacks zip archive members entirely into memory
without validating uncompressed sizes. In `_statement_from_zip` and `_tests_from_zip`,
calling `package.read().decode()` decompresses contents before applying test count
limits. A zip archive with high compression ratios (zip bomb) can decompress several
gigabytes of data into memory across statement files and `.in`/`.ans` pairs, leading to
worker memory exhaustion.
…cts + program output only; stderr/compiler text persists on the StressRun audit row (clears the CodeQL stack-trace-exposure block)
52c923d to
a5ea012
Compare
| time_ms = 1000 | ||
| memory_kb = 262144 | ||
| if isinstance(limits, dict): | ||
| try: | ||
| time_ms = int(float(limits.get("time_seconds", 1)) * 1000) | ||
| except (TypeError, ValueError): | ||
| time_ms = 1000 | ||
| try: | ||
| memory_kb = int(limits.get("memory_limit_bytes", 262144)) // 1024 | ||
| except (TypeError, ValueError): | ||
| memory_kb = 262144 |
There was a problem hiding this comment.
Bug: The importer expects non-standard time_seconds and memory_limit_bytes keys in problem.yaml, causing standard Kattis problem limits to silently fall back to defaults.
Severity: MEDIUM
Suggested Fix
Update the parser in backend/problems/tooling.py to support standard Kattis keys such as time_limit and memory (converting units accordingly, e.g. seconds to ms and MiB to KB), while keeping fallback support for alternative keys if needed, and log or return a warning when falling back to default limits.
Prompt for AI Agent
Review the code at the location below. A potential bug has been identified by an AI
agent. Verify if this is a real issue. If it is, propose a fix; if not, explain why it's
not valid.
Location: backend/problems/tooling.py#L926-L937
Potential issue: The Kattis problem importer reads `limits.time_seconds` and
`limits.memory_limit_bytes` from `problem.yaml`. Standard Kattis packages declare limits
using `time_limit` and `memory` instead. Because these keys do not match standard Kattis
specifications, packages imported will fail to parse the defined limits and silently
fall back to the default limits (1000 ms and 262144 KB). This can cause valid solutions
to encounter unexpected TLE or MLE verdicts without any warning in the API response.
Wave C closure: the two items a full-scope audit of the shipped Wave C PRs (#216, #232–#236, #238) found incomplete against their roadmap design sketches (ROADMAP-COMPETITIVE-2026-10-22 §4.4 JDG-11, §4.9 RO-6 + §7 Wave C line "JDG-11 import/validator tooling").
JDG-11 — problem-preparation toolchain completion (beyond #238's stress runner)
POST /api/v1/problems/tools/validate/— the testlib validator loop: generator makes inputs, validator must exit 0 on valid ones; the first violating input + the validator's message come back so the author fixes the generator before an input becomes a test case. Same role gate,stressthrottle scope and StressRun audit lane as the stress runner.POST /api/v1/problems/tools/generate-outputs/— run a trusted reference over author-supplied inputs (or a generator batch ≤50) and mint input/output pairs; non-AC reference verdicts are reported per input, never silently turned into outputs.POST /api/v1/problems/tools/import-package/(admin/verifier) — problem.yaml via a flat-YAML-subset parser (no PyYAML; deps untouched), statements/en|ro (\titleextraction),data/sample|secret+ legacytests/layouts, zip-slip guards, 15MB/400-entry/200-test caps. Imports land as DRAFT problems wired to the CNT-2 ledger: a GRANTED ImportSource row or an explicitdeclare_own_contentis required.RO-6 — archive browse axis + enforced rights gate + offline bundle
source_contest/source_year/grade_group+import_sourceledger FK) now serializes on problem list/detail AND the public API payloads; writable by authors.?archive=1&source_year=&grade_group=(server-side, mirroring the curriculum axis) + a catalog filter row (toggle/year/group, URL-synced) and provenance chips on rows.GET /api/v1/problems/{id}/offline-bundle/— statement (EN+RO) + limits + provenance + samples as one printable PDF from a dependency-free writer (problems/pdf_bundle.py).Tests
57+ backend tests (validator/generator/importer matrix incl. zip-slip, caps, yaml garbage, ledger gating; PDF skeleton/pagination/escaping; archive axis + serializer roundtrip + public API), 5 vitest cases (chip render, toggle URL round-trip, refinements), e2e
problems.spec.tsarchive flow with mock-server param filtering + real-stack seeding carrying provenance. No migrations, no new env vars.Gates (this worktree):
SECRET_KEY=x DB_PASSWORD=x python -m pytest -q→ 1754 passed, 100% line+branch, 0 warnings · vitest full suite green ·tsc --noEmitclean ·npm run lint0 problems ·npm run buildgreen ·npx playwright test131 passed + 3 skipped.