CHEF-3849: InSpec should exit quickly and clearly if waiver file is malformed/corrupt by Vasu1105 · Pull Request #6644 · inspec/inspec

Vasu1105 · 2023-09-01T11:38:34Z

Description

If you provide a malformed waiver file - one that fails YAML lint, in other words - inspec will continue to run, showing an error as the first control failing with a control source code error.
This buries the error message(Warning) deeply in the InSpec reporting output, which on a large profile may be hundred of kilobytes of text.
It also wastes the user’s time, since on a large profile, we know waiver file is bad immediately, but we doggedly execute the whole profile (which may take hours) before finally returning unusable (incorrect, unwaivered) results.
In case of invalid parameters in the waiver file currently it shows a warning message whereas it should raise the error in case of required parameters are not present in the waiver file.

This fix includes

Detect a malformed waiver file and exit immediately.
Raise error in case of required parameter is not present in the waiver file.
Introduced a new exception class to handle invalid formatting of waiver file.
Updated code to validate the required parameter for yaml and json file for each waived control.

In case of an empty waiver file, extra parameters, or nil header (in CSV formatted) waiver file it will show a warning message that there will not be any behavioral changes.

Related Issue

Types of changes

Bug fix (non-breaking change which fixes an issue)
New content (non-breaking change)
Breaking change (a content change which would break existing functionality or processes)

Checklist:

I have read the CONTRIBUTING document.

netlify · 2023-09-01T11:38:39Z

✅ Deploy Preview for chef-inspec canceled.

Name	Link
🔨 Latest commit	`cd08a75`
🔍 Latest deploy log	https://app.netlify.com/sites/chef-inspec/deploys/64f9ea8a49829900089bcb37

Signed-off-by: Vasu1105 <vasundhara.jagdale@progress.com>

…waiver file Signed-off-by: Vasu1105 <vasundhara.jagdale@progress.com>

Signed-off-by: Vasu1105 <vasundhara.jagdale@progress.com>

…lumn without headers in waiver file Signed-off-by: Vasu1105 <vasundhara.jagdale@progress.com>

Signed-off-by: Vasu1105 <vasundhara.jagdale@progress.com>

…heck. Co-authored-by: Sathish Babu <80091550+sathish-progress@users.noreply.github.com> Signed-off-by: Vasu1105 <vasundhara.jagdale@progress.com>

sathish-progress

LGTM

Nik08

Functionally it looks good to me. Just have some small refactoring suggestions. Thanks

…other functionalities like waiver file Signed-off-by: Vasu1105 <vasundhara.jagdale@progress.com>

Signed-off-by: Vasu1105 <vasundhara.jagdale@progress.com>

…l the data fields as it was not called inside the loop where we are iterating over the data and fetching the headers. Signed-off-by: Vasu1105 <vasundhara.jagdale@progress.com>

…ra parameters Signed-off-by: Vasu1105 <vasundhara.jagdale@progress.com>

…required parameters and extra parameters in waivers file in all format i.e (yaml, json and csv). Signed-off-by: Vasu1105 <vasundhara.jagdale@progress.com>

…ages for waiver file validation Signed-off-by: Vasu1105 <vasundhara.jagdale@progress.com>

Signed-off-by: Vasu1105 <vasundhara.jagdale@progress.com>

… and instead of return data in array it will now return the data in hash Signed-off-by: Vasu1105 <vasundhara.jagdale@progress.com>

…alformed/corrupt (#6644) * Functional test for malformed waiver file Signed-off-by: Vasu1105 <vasundhara.jagdale@progress.com> * Raise error for malformed yaml content and exit Signed-off-by: Vasu1105 <vasundhara.jagdale@progress.com> * Updates functional test for malformed yaml waiver file and for empty waiver file Signed-off-by: Vasu1105 <vasundhara.jagdale@progress.com> * Raises error in case of missing required parameters in waiver file Signed-off-by: Vasu1105 <vasundhara.jagdale@progress.com> * Update functional test for missing parameters, extra parameters or column without headers in waiver file Signed-off-by: Vasu1105 <vasundhara.jagdale@progress.com> * Fix linting Signed-off-by: Vasu1105 <vasundhara.jagdale@progress.com> * Fix warning and error messages Signed-off-by: Vasu1105 <vasundhara.jagdale@progress.com> * Updates nil and false check for yaml data and adds additional empty check. Co-authored-by: Sathish Babu <80091550+sathish-progress@users.noreply.github.com> Signed-off-by: Vasu1105 <vasundhara.jagdale@progress.com> * Adds more generic message as this yaml reader is now getting used by other functionalities like waiver file Signed-off-by: Vasu1105 <vasundhara.jagdale@progress.com> * Fixed test description to reflect correct use case Signed-off-by: Vasu1105 <vasundhara.jagdale@progress.com> * Fix validate headers was not validating the required parametes for all the data fields as it was not called inside the loop where we are iterating over the data and fetching the headers. Signed-off-by: Vasu1105 <vasundhara.jagdale@progress.com> * Updates the test files for the use case to missing parameters and extra parameters Signed-off-by: Vasu1105 <vasundhara.jagdale@progress.com> * Updates code to better handle errors and warnings related to missing required parameters and extra parameters in waivers file in all format i.e (yaml, json and csv). Signed-off-by: Vasu1105 <vasundhara.jagdale@progress.com> * Updated functional test to capture the updated error and warning messages for waiver file validation Signed-off-by: Vasu1105 <vasundhara.jagdale@progress.com> * Fix linting Signed-off-by: Vasu1105 <vasundhara.jagdale@progress.com> * Deleted fixture file which is not required Signed-off-by: Vasu1105 <vasundhara.jagdale@progress.com> * Refactor: Renamed method validate_headers to reflect whats it's doing and instead of return data in array it will now return the data in hash Signed-off-by: Vasu1105 <vasundhara.jagdale@progress.com> --------- Signed-off-by: Vasu1105 <vasundhara.jagdale@progress.com> Co-authored-by: Sathish Babu <80091550+sathish-progress@users.noreply.github.com>

Vasu1105 requested a review from a team as a code owner September 1, 2023 11:38

Vasu1105 requested review from Nik08 and clintoncwolfe September 1, 2023 11:38

Vasu1105 added the Do Not Merge label Sep 1, 2023

Vasu1105 changed the title ~~CHEF-3849: InSpec should exit quickly and clearly if waiver file is malformed/corrupt~~ [W-I-P] CHEF-3849: InSpec should exit quickly and clearly if waiver file is malformed/corrupt Sep 1, 2023

chef-expeditor Bot assigned Vasu1105 Sep 1, 2023

Vasu1105 force-pushed the vj/inspec-should-exit-quickly-if-waivers-file-is-currupt branch from a278a24 to d496b66 Compare September 1, 2023 12:04

Functional test for malformed waiver file

bf7262e

Signed-off-by: Vasu1105 <vasundhara.jagdale@progress.com>

Vasu1105 force-pushed the vj/inspec-should-exit-quickly-if-waivers-file-is-currupt branch from d496b66 to bf7262e Compare September 1, 2023 12:10

Vasu1105 added 4 commits September 4, 2023 13:25

Raise error for malformed yaml content and exit

2072cfc

Signed-off-by: Vasu1105 <vasundhara.jagdale@progress.com>

Updates functional test for malformed yaml waiver file and for empty …

c36cb1f

…waiver file Signed-off-by: Vasu1105 <vasundhara.jagdale@progress.com>

Raises error in case of missing required parameters in waiver file

d8d54f6

Signed-off-by: Vasu1105 <vasundhara.jagdale@progress.com>

Update functional test for missing parameters, extra parameters or co…

6aabedc

…lumn without headers in waiver file Signed-off-by: Vasu1105 <vasundhara.jagdale@progress.com>

Vasu1105 force-pushed the vj/inspec-should-exit-quickly-if-waivers-file-is-currupt branch from 8acca95 to 6aabedc Compare September 4, 2023 10:16

Fix linting

9c9a417

Signed-off-by: Vasu1105 <vasundhara.jagdale@progress.com>

Vasu1105 changed the title ~~[W-I-P] CHEF-3849: InSpec should exit quickly and clearly if waiver file is malformed/corrupt~~ CHEF-3849: InSpec should exit quickly and clearly if waiver file is malformed/corrupt Sep 4, 2023

Vasu1105 removed the Do Not Merge label Sep 4, 2023

sathish-progress reviewed Sep 4, 2023

View reviewed changes

Comment thread lib/inspec/secrets/yaml.rb Outdated

Vasu1105 and others added 2 commits September 5, 2023 20:37

Fix warning and error messages

d6b35ab

Signed-off-by: Vasu1105 <vasundhara.jagdale@progress.com>

Updates nil and false check for yaml data and adds additional empty c…

80f23cc

…heck. Co-authored-by: Sathish Babu <80091550+sathish-progress@users.noreply.github.com> Signed-off-by: Vasu1105 <vasundhara.jagdale@progress.com>

Vasu1105 force-pushed the vj/inspec-should-exit-quickly-if-waivers-file-is-currupt branch from c553131 to 80f23cc Compare September 6, 2023 07:49

sathish-progress approved these changes Sep 6, 2023

View reviewed changes

Nik08 reviewed Sep 6, 2023

View reviewed changes

Comment thread lib/inspec/waiver_file_reader.rb

Nik08 reviewed Sep 6, 2023

View reviewed changes

Comment thread lib/inspec/secrets/yaml.rb Outdated

Nik08 approved these changes Sep 6, 2023

View reviewed changes

Adds more generic message as this yaml reader is now getting used by …

44bc614

…other functionalities like waiver file Signed-off-by: Vasu1105 <vasundhara.jagdale@progress.com>

Vasu1105 force-pushed the vj/inspec-should-exit-quickly-if-waivers-file-is-currupt branch from 9aa2f9e to 44bc614 Compare September 6, 2023 10:06

Vasu1105 added 2 commits September 6, 2023 19:02

Fixed test description to reflect correct use case

9ca944a

Signed-off-by: Vasu1105 <vasundhara.jagdale@progress.com>

Fix validate headers was not validating the required parametes for al…

4d138b3

…l the data fields as it was not called inside the loop where we are iterating over the data and fetching the headers. Signed-off-by: Vasu1105 <vasundhara.jagdale@progress.com>

Vasu1105 added 6 commits September 6, 2023 19:35

Updates the test files for the use case to missing parameters and ext…

dd9dd19

…ra parameters Signed-off-by: Vasu1105 <vasundhara.jagdale@progress.com>

Updates code to better handle errors and warnings related to missing …

c8b5bc4

…required parameters and extra parameters in waivers file in all format i.e (yaml, json and csv). Signed-off-by: Vasu1105 <vasundhara.jagdale@progress.com>

Updated functional test to capture the updated error and warning mess…

c517677

…ages for waiver file validation Signed-off-by: Vasu1105 <vasundhara.jagdale@progress.com>

Fix linting

20fd17e

Signed-off-by: Vasu1105 <vasundhara.jagdale@progress.com>

Deleted fixture file which is not required

6b18739

Signed-off-by: Vasu1105 <vasundhara.jagdale@progress.com>

Refactor: Renamed method validate_headers to reflect whats it's doing…

cd08a75

… and instead of return data in array it will now return the data in hash Signed-off-by: Vasu1105 <vasundhara.jagdale@progress.com>

clintoncwolfe approved these changes Sep 12, 2023

View reviewed changes

clintoncwolfe merged commit 95c17d4 into main Sep 12, 2023

clintoncwolfe deleted the vj/inspec-should-exit-quickly-if-waivers-file-is-currupt branch September 12, 2023 14:36

Vasu1105 restored the vj/inspec-should-exit-quickly-if-waivers-file-is-currupt branch September 13, 2023 06:28

Nik08 mentioned this pull request Mar 27, 2025

Backports: CHEF-3849: InSpec should exit quickly and clearly if waiver file is malformed/corrupt #7345

Merged

4 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

CHEF-3849: InSpec should exit quickly and clearly if waiver file is malformed/corrupt#6644

CHEF-3849: InSpec should exit quickly and clearly if waiver file is malformed/corrupt#6644
clintoncwolfe merged 17 commits intomainfrom
vj/inspec-should-exit-quickly-if-waivers-file-is-currupt

Vasu1105 commented Sep 1, 2023 •

edited

Loading

Uh oh!

netlify Bot commented Sep 1, 2023 •

edited

Loading

Uh oh!

Uh oh!

sathish-progress left a comment

Uh oh!

Uh oh!

Uh oh!

Nik08 left a comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

Conversation

Vasu1105 commented Sep 1, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Related Issue

Types of changes

Checklist:

Uh oh!

netlify Bot commented Sep 1, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

✅ Deploy Preview for chef-inspec canceled.

Uh oh!

Uh oh!

sathish-progress left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Nik08 left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

Vasu1105 commented Sep 1, 2023 •

edited

Loading

netlify Bot commented Sep 1, 2023 •

edited

Loading