Open
Conversation
Accessing catalog['Categories']['Category'] without guards caused an unhandled KeyError if the CAPEC JSON had no Categories section. Added defensive checks consistent with existing guards for Attack_Patterns and Attack_Pattern. Logs a warning and skips the categories block if the key is absent or malformed. Fixes OWASP#2488
…er; preserve behavior for tests
…ame_session cover, player index route test
…te_limiter prod bypass, schema changeset tests
…nd next_round, ip_helper socket branches
…ate_limiter coverage tests
…/1 to fix KeyError
… hacky multi-clause render
Contributor
There was a problem hiding this comment.
Pull request overview
This PR fixes unclosed file handles in Python scripts (convert_asvs.py and convert_capec.py) by converting manual open()/close() patterns to with context managers, ensuring files are properly closed even when exceptions occur. The PR also includes several other fixes and test coverage improvements across both the Python scripts and the Elixir (copi.owasp.org) codebase.
Changes:
- Replaced manual file open/close with
withcontext managers inconvert_asvs.pyandconvert_capec.py, and refactoredcapec_map_enricher.pyto extract a reusable_extract_names_from_itemshelper with improved error handling for missing categories. - Fixed LiveView callback return values in
player_live/show.ex(changed{:ok, redirect(...)}to{:noreply, redirect(...)}) and added a nil guard inindex.html.heexto prevent rendering FormComponent when@playeris nil. - Added extensive test coverage across Python and Elixir codebases, including file handle closure verification tests, pure function tests, edge case tests, and model changeset tests.
Reviewed changes
Copilot reviewed 19 out of 19 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
scripts/convert_asvs.py |
Converted file open/close to with context manager |
scripts/convert_capec.py |
Converted two file open/close blocks to with context managers |
scripts/capec_map_enricher.py |
Extracted _extract_names_from_items helper; added defensive checks for Categories |
tests/scripts/convert_asvs_utest.py |
Added file handle closure assertions and error-handling test |
tests/scripts/convert_capec_utest.py |
Added file handle closure test for CAPEC page generation |
copi.owasp.org/lib/copi_web/live/player_live/show.ex |
Fixed handle_params/handle_info return values from {:ok, ...} to {:noreply, ...} |
copi.owasp.org/lib/copi_web/live/player_live/index.html.heex |
Added nil guard around FormComponent rendering |
copi.owasp.org/test/copi_web/live/player_live/show_test.exs |
Added tests for nonexistent player redirect and validation errors |
copi.owasp.org/test/copi_web/live/player_live/show_pure_test.exs |
New pure function tests for PlayerLive.Show helpers |
copi.owasp.org/test/copi_web/live/game_live/show_test.exs |
Added tests for edge cases including different game id, finished game, and nonexistent game |
copi.owasp.org/test/copi_web/live/game_live/show_pure_test.exs |
New pure function tests for GameLive.Show helpers |
copi.owasp.org/test/copi_web/controllers/card_controller_test.exs |
Added test for format_capec/1 |
copi.owasp.org/test/copi_web/plugs/rate_limiter_plug_test.exs |
Improved test for no-IP scenario; added init/1 test |
copi.owasp.org/test/copi/rate_limiter_test.exs |
Added tests for normalize_ip fallback, prod env bypass, and cleanup |
copi.owasp.org/test/copi/ip_helper_test.exs |
Added tests for various socket/connect_info edge cases |
copi.owasp.org/test/copi/cornucopia/vote_test.exs |
New changeset test for Vote |
copi.owasp.org/test/copi/cornucopia/player_test.exs |
New changeset tests for Player |
copi.owasp.org/test/copi/cornucopia/dealt_card_test.exs |
New changeset and find tests for DealtCard |
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Proposed fix
Describe the bug
In
scripts/convert_asvs.py, thecreate_level_summary()function opens a file usingf = open(...)without awithcontext manager. If any exception is raised inside the loop body (e.g., a missing dictionary key during iteration), the file handlefis never explicitly closed, leaking the OS file descriptor. On batch runs that generate many ASVS taxonomy pages, this can exhaust system file handle limits.