-
Notifications
You must be signed in to change notification settings - Fork 19
Fix issue with missing system data in the arf report #216
Conversation
def generate_arf_report_without_system_data(): | ||
arf_report_path = get_arf_report_path() | ||
data = None | ||
with open(arf_report_path, "r", encoding="utf-8") as arf_report: | ||
data = arf_report.readlines() | ||
tmp_patch = Path(tempfile.gettempdir()) / "arf_report_without_system_data.xml" | ||
if tmp_patch.exists(): | ||
return str(tmp_patch) | ||
with open(tmp_patch, "w", encoding="utf-8") as new_arf_report: | ||
skip = False | ||
for row in data: | ||
if "<system_data>" in row: | ||
skip = True | ||
if not skip: | ||
new_arf_report.write(row) | ||
if "</system_data>" in row: | ||
skip = False | ||
return str(tmp_patch) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I love that you have created a test. But I find this way of testing little crazy. You take a 12 MB ARF file, manually remove some lines from there, which effectively creates an invalid or nonsense file and then you test something on that. I guess people will have a hard time understanding this test. If I would write a test for it, I would create a very minimalist SCAP source data stream which would contain just a single simple rule, no additional bells and whistles, and then I would evaluate it so that the generated ARF wouldn't contain any collected object which would lead to missing system_data element and then I would put this file to the repository. The rule can be anything simple like requiring a non-existing file.
( | ||
"xccdf_org.ssgproject.content_rule_accounts_passwords_pam_faillock_deny", | ||
"false", | ||
), | ||
( | ||
"xccdf_org.ssgproject.content_rule_sshd_disable_gssapi_auth", | ||
"false", | ||
), | ||
( | ||
"xccdf_org.ssgproject.content_rule_service_debug-shell_disabled", | ||
"true", | ||
), | ||
( | ||
"xccdf_org.ssgproject.content_rule_mount_option_dev_shm_noexec", | ||
"false", | ||
), | ||
( | ||
"xccdf_org.ssgproject.content_rule_audit_rules_unsuccessful_file_modification_creat", | ||
"false", | ||
), | ||
( | ||
"xccdf_org.ssgproject.content_rule_audit_rules_file_deletion_events_rmdir", | ||
"false", | ||
), | ||
( | ||
"xccdf_org.ssgproject.content_rule_require_singleuser_auth", | ||
"true", | ||
), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What is the intent behind testing all these rule IDs here?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
These rules contain all common cases of OVAL trees.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's a great improvement and surely a more concise test.
This PR fixes issue with missing system data in the arf report and creates basic tests for this issue.
Fixes #204