Skip to content

Commit

Permalink
fix: don't accumulate result state with rhel provider (#253)
Browse files Browse the repository at this point in the history 
* fix: don't accumulate result state with rhel provider

Sets the result store policy for the `rhel` provider to
`DELETE_BEFORE_WRITE`.  There shouldn't be a reason for the rhel
provider to need to accumulate result state between runs as it always
iterates over the full input set.  This will address an issue where
vulnerabilities marked as `not-affected` don't get dropped from the
results.

Signed-off-by: Weston Steimel <[email protected]>

* fix: merge out of support entries when affected or fixed version in more recent release

Signed-off-by: Weston Steimel <[email protected]>

* chore: bump label dataset

Signed-off-by: Weston Steimel <[email protected]>

---------

Signed-off-by: Weston Steimel <[email protected]>
  • Loading branch information
@westonsteimel
westonsteimel authored Jul 27, 2023
1 parent 044fcdf commit 9977f6d
Show file tree
Hide file tree
Showing 5 changed files with 77 additions and 21 deletions.
2 changes: 1 addition & 1 deletion src/vunnel/providers/rhel/__init__.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ class Config:
runtime: provider.RuntimeConfig = field(
default_factory=lambda: provider.RuntimeConfig(
result_store=result.StoreStrategy.SQLITE,
existing_results=result.ResultStatePolicy.KEEP,
existing_results=result.ResultStatePolicy.DELETE_BEFORE_WRITE,
),
)
request_timeout: int = 125
Expand Down
35 changes: 21 additions & 14 deletions src/vunnel/providers/rhel/parser.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -401,7 +401,7 @@ def _get_name_version(package):

return name, version

def _parse_affected_release(self, cve_id, content):
def _parse_affected_release(self, cve_id: str, content) -> list[FixedIn]:
fixed_ins = []
ars = content.get("affected_release", [])

Expand Down Expand Up @@ -563,9 +563,9 @@ def _parse_affected_release(self, cve_id, content):

return fixed_ins

def _parse_package_state(self, cve_id, content):
fixed_ins = []
out_of_support = [] # Track items out of support to be able to add them if others are affected
def _parse_package_state(self, cve_id: str, fixed: list[FixedIn], content) -> list[FixedIn]:
affected: list[FixedIn] = []
out_of_support: list[FixedIn] = [] # Track items out of support to be able to add them if others are affected
pss = content.get("package_state", [])

for item in pss:
Expand Down Expand Up @@ -595,7 +595,7 @@ def _parse_package_state(self, cve_id, content):

state = item.get("fix_state", None)
if state in ["Affected", "Fix deferred"]:
fixed_ins.append(
affected.append(
FixedIn(
platform=platform,
package=package_name,
Expand All @@ -605,7 +605,7 @@ def _parse_package_state(self, cve_id, content):
)
)
elif state in ["Will not fix"]:
fixed_ins.append(
affected.append(
FixedIn(
platform=platform,
package=package_name,
Expand Down Expand Up @@ -636,34 +636,41 @@ def _parse_package_state(self, cve_id, content):
except:
self.logger.exception(f"error parsing {cve_id} package state entity: {item}")

merged_fixed_ins = Parser._merge_out_of_support_affected(fixed_ins, out_of_support)
merged_fixed_ins = Parser._merge_out_of_support_affected(fixed, affected, out_of_support)
return merged_fixed_ins

@staticmethod
def _merge_out_of_support_affected(affected_fixed_ins: list[FixedIn], out_of_support: list[FixedIn]) -> list[FixedIn]:
if out_of_support and affected_fixed_ins:
merged = copy.deepcopy(affected_fixed_ins)
def _merge_out_of_support_affected(
fixed: list[FixedIn], affected: list[FixedIn], out_of_support: list[FixedIn]
) -> list[FixedIn]:
if not out_of_support:
return affected

if affected or fixed:
merged = copy.deepcopy(affected)

for oos in out_of_support:
for affected in affected_fixed_ins:
for r in affected + fixed:
# A newer release is impacted, so assume out-of-support version is as well
try:
if oos.package == affected.package and int(oos.platform) < int(affected.platform):
if oos.package == r.package and int(oos.platform) < int(r.platform):
merged.append(oos)
break
except ValueError:
# Be conservative if we cannot tell if it is <
merged.append(oos)
break
return merged
return affected_fixed_ins

return affected

def _parse_cve(self, cve_id, content):
# logger.debug('Parsing {}'.format(cve_id))

results = []
platform_artifacts = {}
fins = self._parse_affected_release(cve_id, content)
nfins = self._parse_package_state(cve_id, content)
nfins = self._parse_package_state(cve_id, fins, content)
platform_package_module_tuples = set()

if fins or nfins:
Expand Down
2 changes: 1 addition & 1 deletion tests/quality/vulnerability-match-labels
Submodule vulnerability-match-labels updated 220 files
2 changes: 1 addition & 1 deletion tests/unit/cli/test_cli.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -241,7 +241,7 @@ def test_config(monkeypatch) -> None:
request_timeout: 125
runtime:
existing_input: keep
existing_results: keep
existing_results: delete-before-write
on_error:
action: fail
input: keep
Expand Down
57 changes: 53 additions & 4 deletions tests/unit/providers/rhel/test_rhel.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -444,7 +444,7 @@ def test_parse_affected_releases(self, tmpdir, affected_releases, fixed_ins, moc

def test_parse_package_state(self, tmpdir, mock_cve):
driver = Parser(workspace=workspace.Workspace(tmpdir, "test", create=True))
results = driver._parse_package_state(mock_cve.get("name"), mock_cve)
results = driver._parse_package_state([], mock_cve.get("name"), mock_cve)

assert results and isinstance(results, list) and len(results) == 1
fixed_in = results[0]
Expand Down Expand Up @@ -512,9 +512,10 @@ def test_get_name_version(self, package, name, version):


@pytest.mark.parametrize(
"affected, out_of_support, expected",
"fixed, affected, out_of_support, expected",
[
(
[],
[
FixedIn(
module=None,
Expand Down Expand Up @@ -551,6 +552,7 @@ def test_get_name_version(self, package, name, version):
],
),
(
[],
[
FixedIn(
module=None,
Expand Down Expand Up @@ -580,6 +582,7 @@ def test_get_name_version(self, package, name, version):
],
),
(
[],
[
FixedIn(
module=None,
Expand All @@ -601,6 +604,7 @@ def test_get_name_version(self, package, name, version):
],
),
(
[],
[],
[
FixedIn(
Expand All @@ -614,6 +618,7 @@ def test_get_name_version(self, package, name, version):
[],
),
(
[],
[
FixedIn(
module=None,
Expand Down Expand Up @@ -642,10 +647,54 @@ def test_get_name_version(self, package, name, version):
),
],
),
(
[
FixedIn(
module=None,
platform="8",
package="foobar",
advisory=None,
version="1.2.3.4",
)
],
[],
[
FixedIn(
module=None,
platform="6",
package="foobar",
advisory=None,
version=None,
),
FixedIn(
module=None,
platform="7",
package="foobar",
advisory=None,
version=None,
),
],
[
FixedIn(
module=None,
platform="6",
package="foobar",
advisory=None,
version=None,
),
FixedIn(
module=None,
platform="7",
package="foobar",
advisory=None,
version=None,
),
],
),
],
)
def test_out_of_support(affected, out_of_support, expected):
assert Parser._merge_out_of_support_affected(affected, out_of_support) == expected
def test_out_of_support(fixed, affected, out_of_support, expected):
assert Parser._merge_out_of_support_affected(fixed, affected, out_of_support) == expected


@pytest.fixture
Expand Down

0 comments on commit 9977f6d

Please sign in to comment.