Skip to content

fix(terraform): handle hanging Terraform file parsing with process-based timeout #6871

New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

MS99-9
Copy link

@MS99-9 MS99-9 commented Nov 24, 2024

User description

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

Description

This PR introduces a ProcessTimeout class to handle cases where Terraform file parsing in Checkov hangs indefinitely, particularly when using the -d flag to scan directories. The ProcessTimeout class uses subprocesses to enforce timeouts for blocking operations, complementing the existing SignalTimeout and ThreadingTimeout mechanisms. Additionally, updates were made to __parse_with_timeout to address race conditions.

Changes include:

  1. Added ProcessTimeout:
  • Introduced a new timeout mechanism that uses subprocesses to execute potentially blocking operations, allowing them to be forcibly terminated if they exceed the specified timeout duration.
  • Serves as a fallback for non-main threads and blocking operations where SignalTimeout (Unix signals) and ThreadingTimeout (used on Windows) are ineffective.
  1. Updated __parse_with_timeout:
  • Integrated ProcessTimeout as an additional timeout mechanism to handle cases where file parsing would hang indefinitely due to blocking operations or syntax errors in Terraform files.
  • The issue occurred because SignalTimeout only works in the main thread and ThreadingTimeout cannot interrupt blocking operations due to Python’s Global Interpreter Lock. As a result, Checkov was unable to terminate the parsing process when using the -d flag.
  • Addressed potential race conditions by ensuring the setup_interrupt process for ProcessTimeout completes before execution proceeds.

Testing

Before the fix, running Checkov with a syntax error in the Terraform file would result in Checkov hanging indefinitely. After the fix, Checkov terminates the process and logs the parsing error properly. Example output:

      _               _              
   ___| |__   ___  ___| | _______   __
  / __| '_ \ / _ \/ __| |/ / _ \ \ / /
 | (__| | | |  __/ (__|   < (_) \ V / 
  \___|_| |_|\___|\___|_|\_\___/ \_/  
                                      
By Prisma Cloud | version: 3.2.313 

terraform scan results:

Passed checks: 0, Failed checks: 0, Skipped checks: 0, Parsing errors: 1

Error parsing file <failed_file_path>

Running Pytests and coverage modules pipenv run python -m coverage run -m pytest tests:

=========================================== test session starts ============================================
platform darwin -- Python 3.10.14, pytest-7.4.4, pluggy-1.5.0
rootdir: /Users/mohamedshalaby/dev/ms99-9/checkov
configfile: pyproject.toml
plugins: asyncio-0.23.8, cov-5.0.0, time-machine-2.15.0, mock-3.14.0, xdist-3.6.1
asyncio: mode=strict
2 workers [4689 items]  
.................................................................................................... [  2%]
.................................................................................................... [  4%]
.................................................................................................... [  6%]
.................................................................................................... [  8%]
.........................................s..........................s............................... [ 10%]
.................................................................................................... [ 12%]
.......................................s...................s......s................................. [ 14%]
.................................................................................................... [ 17%]
.................................................................................................... [ 19%]
.................................................................................................... [ 21%]
.................................................................................................... [ 23%]
.................................................................................................... [ 25%]
.................................................................................................... [ 27%]
.................................................................................................... [ 29%]
.................................................................................................... [ 31%]
.................................................................................................... [ 34%]
.................................................................................................... [ 36%]
.................................................................................................... [ 38%]
.................................................................................................... [ 40%]
.................................................................................................... [ 42%]
.............................................................................................sssssss [ 44%]
.................................................................................................... [ 46%]
.................................................................................................... [ 49%]
..............................................................ss.................................... [ 51%]
.................................................................................................... [ 53%]
.............................................ssss............sss.................................... [ 55%]
.................................................................................................... [ 57%]
.................................................................................................... [ 59%]
...s................................................................................................ [ 61%]
.................................................................................................... [ 63%]
.................................................................................................... [ 66%]
.............................ssss................................................................... [ 68%]
.................................................................................................... [ 70%]
.................................................................................................... [ 72%]
.................................................................................................... [ 74%]
.................................................................................................... [ 76%]
.................................................................................................... [ 78%]
.................................................................................................... [ 81%]
....................................s............................................................... [ 83%]
.................................................................................................... [ 85%]
.................................................................................................... [ 87%]
.................................................................................................... [ 89%]
.................................................................................................... [ 91%]
.................................................................................................... [ 93%]
.................................................................................................... [ 95%]
.................................................................................................... [ 98%]
.........................................................................................            [100%]
======================== 4662 passed, 27 skipped, 11 warnings in 885.55s (0:14:45) =========================

Files created while testing

Fixes #6762

Questions

  1. Should I increment the version in checkov/version.py?
  2. I didn't find any tests for the checkov/common/util/stopit classes, should I create test files for the timeout classes? maybe another PR for this [enhancement]
  3. Can you please check if the introduced changes don't alter the logic for parsing Terraform files, I already built the package locally with the changes and tested multiple cases but my concern if I missed any of the edge cases.

Checklist:

  • I have performed a self-review of my own code
  • I have commented my code, particularly in hard-to-understand areas
  • I have made corresponding changes to the documentation
  • I have added tests that prove my feature, policy, or fix is effective and works
  • New and existing tests pass locally with my changes

Generated description

Below is a concise technical summary of the changes proposed in this PR:

Introduces a new ProcessTimeout class to handle cases where Terraform file parsing hangs indefinitely. Updates the __parse_with_timeout function to use ProcessTimeout as a fallback mechanism for non-main threads and blocking operations. This enhancement complements the existing SignalTimeout and ThreadingTimeout mechanisms, addressing race conditions and improving the robustness of the parsing process.

TopicDetails
ProcessTimeout Implements a new ProcessTimeout class using multiprocessing to enforce timeouts for blocking operations
Modified files (2)
  • checkov/common/util/stopit/processstop.py
  • checkov/common/util/stopit/__init__.py
Latest Contributors(2)
UserCommitDate
gruebelchore-remove-unneeded-...February 23, 2023
omryMenfeat-secrets-add-timeo...February 22, 2023
ParseWithTimeout Updates __parse_with_timeout function to integrate ProcessTimeout and handle various timeout scenarios
Modified files (1)
  • checkov/terraform/tf_parser.py
Latest Contributors(2)
UserCommitDate
rotemavnifix-terraform-Set-time...October 13, 2024
ChanochShaynerfeat-terraform-Add-pro...May 29, 2024
This pull request is reviewed by Baz. Join @MS99-9 and the rest of your team on (Baz).

@MS99-9
Copy link
Author

MS99-9 commented Dec 18, 2024

Is there anything missing in the PR that I need to provide in order for it to get reviewed? please let me know

Signed-off-by: Mohamed Medhat Mohamed Ibrahim Shalaby <mohamed.shalaby@tum.de>
@MS99-9
Copy link
Author

MS99-9 commented Dec 30, 2024

Hello @bo156 @SteveVaknin, I am mentioning you because I saw you were common reviewers of previous PRs. I have been waiting a while for this PR to be reviewed, so please let me know if I am missing a step in order to get the PR reviewed faster. Thank you

# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Chekov hangs during scanning terraform module with syntax typo
1 participant