fix: guard against missing Categories key in extract_capec_names#2506
fix: guard against missing Categories key in extract_capec_names#2506immortal71 wants to merge 16 commits intoOWASP:masterfrom
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
There was a problem hiding this comment.
Pull request overview
Fixes a crash in scripts/capec_map_enricher.py by adding defensive handling when CAPEC JSON lacks the Categories / Category structure, aligning with the function’s earlier guard patterns.
Changes:
- Add guards around
catalog["Categories"]["Category"]access to avoidKeyErrorwhenCategoriesis missing. - Log warnings and continue processing when
Categories/Categoryis absent orCategoryis not a list.
scripts/capec_map_enricher.py
Outdated
| elif "Category" not in catalog["Categories"]: | ||
| logging.warning("No 'Category' key found in categories section") | ||
| elif not isinstance(catalog["Categories"]["Category"], list): | ||
| logging.warning("'Category' is not a list") | ||
| else: | ||
| for category in catalog["Categories"]["Category"]: | ||
| if "_ID" in category and "_Name" in category: | ||
| capec_id = int(category["_ID"]) | ||
| capec_name = category["_Name"] | ||
| capec_names[capec_id] = capec_name |
There was a problem hiding this comment.
elif "Category" not in catalog["Categories"] will still raise a TypeError if catalog["Categories"] is None or another non-iterable non-mapping value (malformed CAPEC JSON). Consider first validating catalog["Categories"] is a dict (or at least a Mapping) before doing membership tests / indexing, and then validate the Category value type.
| elif "Category" not in catalog["Categories"]: | |
| logging.warning("No 'Category' key found in categories section") | |
| elif not isinstance(catalog["Categories"]["Category"], list): | |
| logging.warning("'Category' is not a list") | |
| else: | |
| for category in catalog["Categories"]["Category"]: | |
| if "_ID" in category and "_Name" in category: | |
| capec_id = int(category["_ID"]) | |
| capec_name = category["_Name"] | |
| capec_names[capec_id] = capec_name | |
| else: | |
| categories = catalog["Categories"] | |
| if not isinstance(categories, dict): | |
| logging.warning("'Categories' section is not a dictionary; skipping category-based CAPEC names") | |
| elif "Category" not in categories: | |
| logging.warning("No 'Category' key found in categories section") | |
| elif not isinstance(categories["Category"], list): | |
| logging.warning("'Category' is not a list") | |
| else: | |
| for category in categories["Category"]: | |
| if "_ID" in category and "_Name" in category: | |
| capec_id = int(category["_ID"]) | |
| capec_name = category["_Name"] | |
| capec_names[capec_id] = capec_name |
scripts/capec_map_enricher.py
Outdated
| if "Categories" not in catalog: | ||
| logging.warning("No 'Categories' key found in catalog") | ||
| elif "Category" not in catalog["Categories"]: | ||
| logging.warning("No 'Category' key found in categories section") | ||
| elif not isinstance(catalog["Categories"]["Category"], list): | ||
| logging.warning("'Category' is not a list") | ||
| else: | ||
| for category in catalog["Categories"]["Category"]: | ||
| if "_ID" in category and "_Name" in category: | ||
| capec_id = int(category["_ID"]) | ||
| capec_name = category["_Name"] | ||
| capec_names[capec_id] = capec_name |
There was a problem hiding this comment.
There are unit tests for missing Attack_Pattern_Catalog / Attack_Patterns / Attack_Pattern, but no test coverage for the newly added Categories guards (missing Categories, missing Category, or non-list Category). Adding targeted tests would prevent regressions on this crash path.
Extracted item-loop logic into a private helper function to bring extract_capec_names cyclomatic complexity back within flake8 C901 limit (max 10). Fixes CI failure on complexity check.
Covers the three new guard paths added in the fix: - Missing Categories key (warns, still returns attack patterns) - Missing Category key inside Categories (warns) - Non-list Category value (silently skipped by helper)
The helper silently skips non-list values, breaking the existing test that expects a warning log when Attack_Pattern is not a list. Restored explicit guard with early return before calling the helper.
Targets uncovered paths in GameLive.Show and PlayerLive.Show: - start_game: already started (noop) and < 3 players (error flash) - invalid round param redirect - display_game_session / latest_version for all editions - card_played_in_round helper - next_round with closed round - toggle_vote removal path - redirect to error when player not found - Pure helper functions: ordered_cards, unplayed_cards, played_cards, player_first, round_open?, round_closed?, last_round?, get_vote
- router.ex: add player edit route (/players/:id/edit -> PlayerLive.Index :edit) and wrap game show in live_session with on_mount registration so GameLive.Show.on_mount/4 and put_uri_hook/3 are exercised by all existing game show tests - rate_limiter_plug_test: add init/1 assertion (was called by router but not by direct Plug.Test calls in the test suite) - form_component_test: add describe block for edit path covering save_player(:edit) success and changeset error branches
…estore 90 coverage threshold
Issue: #2488
Problem
extract_capec_names() in scripts/capec_map_enricher.py accessed catalog['Categories']['Category'] directly without any guard, causing an unhandled KeyError crash if the CAPEC JSON has no Categories section.
Fix
Added defensive checks matching the existing guard style used for Attack_Patterns and Attack_Pattern earlier in the same function. If Categories or Category is absent/malformed, a warning is logged and the block is skipped — the function continues and returns whatever attack pattern names were already extracted.