fix(terraform_plan): add a check to avoid doing get on a none dict object in tfplan scan #7180
Conversation
checkov/terraform/plan_runner.py
Outdated
| resource_type_dict = entity.get(resource_type, {}) | ||
| entity_id = resource_type_dict.get(resource_name, resource_type_dict).get(TF_PLAN_RESOURCE_ADDRESS) | ||
| resource_dict = resource_type_dict.get(resource_name, resource_type_dict) | ||
| entity_id: str |
There was a problem hiding this comment.
didn't know you could declate it like that 😅
checkov/terraform/plan_utils.py
Outdated
| resource_type_dict = entity.get(resource_type, {}) | ||
| entity_id = resource_type_dict.get(resource_name, resource_type_dict).get(TF_PLAN_RESOURCE_ADDRESS) | ||
| resource_dict = resource_type_dict.get(resource_name, resource_type_dict) | ||
| if isinstance(resource_dict, dict): |
There was a problem hiding this comment.
seems like that's the same code twice so extract it to a function :)
also - can you add a test for the tf plan which needed that change?
| else: | ||
| entity_id = resource_type_dict.get(TF_PLAN_RESOURCE_ADDRESS) | ||
| if not entity_id: | ||
| raise Exception(f'Failed get_entity_id: {resource_name} does not have {TF_PLAN_RESOURCE_ADDRESS}') |
There was a problem hiding this comment.
Do you want to crash here? It appears this changes the old behavior which theoretically allowed None values
There was a problem hiding this comment.
We can allow None values but it can lead to unexpected behavior. WDYT?
There was a problem hiding this comment.
The main issue with throwing an exception here is that we fail the entire definitions context building, not just for this entity. If you do decide to throw an exception here, perhaps you can catch it on the outside (log warning) and continue to the next block ?
| if not isinstance(resource_definition, dict): | ||
| entity_context['start_line'] = resource_type_dict['start_line'][0] | ||
| entity_context['end_line'] = resource_type_dict['end_line'][0] | ||
| entity_context["code_lines"] = definitions_raw[full_file_path][ | ||
| entity_context["start_line"]: entity_context["end_line"] | ||
| ] | ||
| entity_context['address'] = resource_type_dict[TF_PLAN_RESOURCE_ADDRESS] | ||
| return entity_context |
There was a problem hiding this comment.
Seems this logic is identical to the one we're running in the "elif" clause, can you extract that into a function and use it from both clauses ?
…ject in tfplan scan (#7180) * add a check to avoid doing get on a none dict object
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.
Description
Please include a summary of the change and which issue is fixed. Please also include relevant motivation and context. List any dependencies that are required for this change.
Fixes # (issue)
New/Edited policies (Delete if not relevant)
Description
Include a description of what makes it a violation and any relevant external links.
Fix
How does someone fix the issue in code and/or in runtime?
Checklist: