Skip to content
New issue

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

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

Already on GitHub? Sign in to your account

Support for spaces in policy reference_id #833

Merged

Conversation

nasir-rabbani
Copy link
Contributor

  1. Updated regex to skip rules with spaces in reference_id.
  2. Added test cases for reference_id with spaces

@nasir-rabbani nasir-rabbani linked an issue Jun 3, 2021 that may be closed by this pull request
@codecov
Copy link

codecov bot commented Jun 3, 2021

Codecov Report

Merging #833 (daacad6) into master (95aba12) will decrease coverage by 0.02%.
The diff coverage is 100.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #833      +/-   ##
==========================================
- Coverage   78.22%   78.20%   -0.03%     
==========================================
  Files         162      164       +2     
  Lines        4354     4359       +5     
==========================================
+ Hits         3406     3409       +3     
- Misses        734      735       +1     
- Partials      214      215       +1     
Impacted Files Coverage Δ
pkg/utils/skip_rules.go 93.33% <100.00%> (-0.42%) ⬇️
pkg/policy/opa/engine.go 59.36% <0.00%> (-5.04%) ⬇️
pkg/iac-providers/tfplan/v1/load-file.go 74.19% <0.00%> (-3.59%) ⬇️
pkg/policy/types.go 100.00% <0.00%> (ø)
pkg/iac-providers/output/types.go 100.00% <0.00%> (ø)
pkg/filters/filter-specs.go 92.30% <0.00%> (ø)
pkg/filters/filters.go 100.00% <0.00%> (ø)
pkg/runtime/executor.go 89.65% <0.00%> (+0.08%) ⬆️
pkg/utils/policy.go 70.00% <0.00%> (+20.00%) ⬆️

Copy link
Contributor

@devang-gaur devang-gaur left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd be great if you could also add some e2e tests for this. since this is a functional change/enhancement.

pkg/utils/skip_rules.go Outdated Show resolved Hide resolved
pkg/utils/skip_rules.go Outdated Show resolved Hide resolved
Copy link
Contributor

@patilpankaj212 patilpankaj212 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi @nasir-rabbani, I think we can now add test in the policies package and verify that all the reference id's / id's pass the regex.

@nasir-rabbani nasir-rabbani force-pushed the fix/skip_ruleid_with_space branch 2 times, most recently from b4a587f to 5309fe0 Compare June 4, 2021 10:50
@nasir-rabbani nasir-rabbani force-pushed the fix/skip_ruleid_with_space branch from 5309fe0 to daacad6 Compare June 4, 2021 11:02
@sonarqubecloud
Copy link

sonarqubecloud bot commented Jun 4, 2021

Kudos, SonarCloud Quality Gate passed!

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 3 Code Smells

No Coverage information No Coverage information
0.0% 0.0% Duplication

Copy link
Contributor

@patilpankaj212 patilpankaj212 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we can push this PR for now, but when we add tests for policies, we would need to ensure that spaces are not allowed in the rule ID / reference ID.

@patilpankaj212 patilpankaj212 merged commit 82e5242 into tenable:master Jun 4, 2021
@nasir-rabbani nasir-rabbani deleted the fix/skip_ruleid_with_space branch August 13, 2021 16:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Rule IDs with spaces cannot be skipped
3 participants