Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions secator/config.py
Original file line number Diff line number Diff line change
Expand Up @@ -201,6 +201,7 @@ class MongodbAddon(StrictModel):
'is_false_positive',
'is_acknowledged',
'verified',
'status',
'tags',
]

Expand All @@ -216,6 +217,7 @@ class SqliteAddon(StrictModel):
'is_false_positive',
'is_acknowledged',
'verified',
'status',
'tags',
]

Expand Down
1 change: 1 addition & 0 deletions secator/definitions.py
Original file line number Diff line number Diff line change
Expand Up @@ -142,6 +142,7 @@
SOURCES = 'sources'
STORED_RESPONSE_PATH = 'stored_response_path'
STATE = 'state'
STATUS = 'status'
STATUS_CODE = 'status_code'
STRING = 'str'
TAGS = 'tags'
Expand Down
20 changes: 18 additions & 2 deletions secator/hooks/_dedup.py
Original file line number Diff line number Diff line change
@@ -1,6 +1,20 @@
# secator/hooks/_dedup.py


def _is_unset(field, value):
"""Return True if `value` should be treated as "empty" for copy-forward purposes.

For most fields, emptiness is the generic falsy check (`not value`). The `status`
field (Vulnerability) is special: its default value `'NEW'` is truthy but means
"untouched", so we treat `''` / `None` / `'NEW'` as unset. This lets a prior
`ACKNOWLEDGED` / `FIXED` status carry forward onto a re-found main whose status is
still the default `'NEW'`, while a never-touched vuln stays `'NEW'`.
"""
if field == 'status':
return not value or str(value).strip().upper() == 'NEW'
return not value


def compute_duplicate_updates(workspace_findings, untagged_findings, copy_fields=None):
"""Compute duplicate-tagging updates for a set of findings (backend-agnostic).

Expand Down Expand Up @@ -35,10 +49,12 @@ def compute_duplicate_updates(workspace_findings, untagged_findings, copy_fields
if not hasattr(previous_item, field):
continue
value_prev = getattr(previous_item, field)
if not value_prev:
# Nothing meaningful to carry forward (handles `status='NEW'` as unset too).
if _is_unset(field, value_prev):
continue
value_curr = getattr(item, field, None)
if not value_curr and field not in copied_fields:
# Copy only onto an "empty" current value; for `status`, `'NEW'` counts as empty.
if _is_unset(field, value_curr) and field not in copied_fields:
copied_fields[field] = value_prev

related_ids = []
Expand Down
13 changes: 11 additions & 2 deletions secator/output_types/vulnerability.py
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,9 @@
from dataclasses import dataclass, field
from typing import List

from secator.definitions import CONFIDENCE, CVSS_SCORE, EXTRA_DATA, ID, MATCHED_AT, NAME, REFERENCE, SEVERITY, TAGS
from secator.definitions import (
CONFIDENCE, CVSS_SCORE, EXTRA_DATA, ID, MATCHED_AT, NAME, REFERENCE, SEVERITY, STATUS, TAGS
)
from secator.output_types import OutputType
from secator.utils import rich_to_ansi, rich_escape as _s, format_object, trim_string

Expand Down Expand Up @@ -30,6 +32,7 @@ class Vulnerability(OutputType):
verified: bool = field(default=False, compare=False)
is_false_positive: bool = field(default=False, compare=False)
is_acknowledged: bool = field(default=False, compare=False)
status: str = field(default='NEW', compare=False)
tags: list = field(default_factory=list, compare=False)
_source: str = field(default='', repr=True, compare=False)
_type: str = field(default='vulnerability', repr=True)
Expand All @@ -40,7 +43,8 @@ class Vulnerability(OutputType):
_duplicate: bool = field(default=False, repr=True, compare=False)
_related: list = field(default_factory=list, compare=False)

_table_fields = [MATCHED_AT, SEVERITY, CONFIDENCE, NAME, ID, CVSS_SCORE, TAGS, EXTRA_DATA, REFERENCE]
_table_fields = [MATCHED_AT, SEVERITY, CONFIDENCE, NAME, ID, CVSS_SCORE, STATUS, TAGS, EXTRA_DATA, REFERENCE]
STATUSES = ('NEW', 'ACKNOWLEDGED', 'FIXED')
_sort_by = ('confidence_nb', 'severity_nb', 'matched_at', 'cvss_score')

@staticmethod
Expand All @@ -66,6 +70,11 @@ def __post_init__(self):
self.severity_nb = severity_map.get(self.severity, 6)
self.confidence_nb = severity_map[self.confidence]

# Normalize status: coerce empty / None / unknown values to 'NEW', uppercase.
# Allowed values are NEW / ACKNOWLEDGED / FIXED (see STATUSES).
status = (self.status or '').strip().upper()
self.status = status if status in self.STATUSES else 'NEW'
Comment on lines +73 to +76

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win

Preserve legacy is_acknowledged state when status is absent.

Line 75-Line 76 always normalizes an unset/invalid status to NEW. That breaks upgrade compatibility for persisted findings created before this field existed: records with is_acknowledged=True but no status now deserialize as NEW, and compute_duplicate_updates() then treats that as unset instead of carrying ACKNOWLEDGED onto the re-found main.

💡 Proposed fix
-		status = (self.status or '').strip().upper()
-		self.status = status if status in self.STATUSES else 'NEW'
+		status = (self.status or '').strip().upper()
+		if status not in self.STATUSES or (status == 'NEW' and self.is_acknowledged):
+			status = 'ACKNOWLEDGED' if self.is_acknowledged else 'NEW'
+		self.status = status

A regression test around a legacy Vulnerability(is_acknowledged=True) load path would lock this down.

📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
# Normalize status: coerce empty / None / unknown values to 'NEW', uppercase.
# Allowed values are NEW / ACKNOWLEDGED / FIXED (see STATUSES).
status = (self.status or '').strip().upper()
self.status = status if status in self.STATUSES else 'NEW'
# Normalize status: coerce empty / None / unknown values to 'NEW', uppercase.
# Allowed values are NEW / ACKNOWLEDGED / FIXED (see STATUSES).
status = (self.status or '').strip().upper()
if status not in self.STATUSES or (status == 'NEW' and self.is_acknowledged):
status = 'ACKNOWLEDGED' if self.is_acknowledged else 'NEW'
self.status = status
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@secator/output_types/vulnerability.py` around lines 73 - 76, Preserve legacy
acknowledgment state in Vulnerability deserialization: the current status
normalization in Vulnerability.__init__ (or the status handling path) always
falls back to NEW when status is missing, which breaks persisted records that
only have is_acknowledged set. Update the normalization logic so that when
status is absent or invalid, it maps is_acknowledged=True to ACKNOWLEDGED and
only defaults to NEW otherwise, while still uppercasing and validating against
STATUSES. Add a regression test for loading a legacy Vulnerability with
is_acknowledged=True and no status, and verify compute_duplicate_updates()
carries ACKNOWLEDGED onto the re-found main.


def __rich__(self):
data = self.extra_data

Expand Down
58 changes: 58 additions & 0 deletions tests/unit/test_dedup.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,58 @@
import unittest

from secator.hooks._dedup import compute_duplicate_updates
from secator.output_types import Vulnerability


def _vuln(uuid, status='NEW', verified=False, **kwargs):
return Vulnerability(
name='CVE-2025-53020',
id='CVE-2025-53020',
matched_at='host:80',
status=status,
verified=verified,
_uuid=uuid,
**kwargs,
)


class TestComputeDuplicateUpdates(unittest.TestCase):

def test_status_carried_forward_onto_new_main(self):
"""A prior ACKNOWLEDGED main carries onto a re-found main whose status is NEW."""
prev = _vuln('prev', status='ACKNOWLEDGED')
new = _vuln('new', status='NEW')
updates = compute_duplicate_updates([prev], [new], copy_fields=['status'])
assert updates['new']['status'] == 'ACKNOWLEDGED'

def test_status_fixed_not_overwritten(self):
"""A new main that is already FIXED keeps its value (FIXED is not 'unset')."""
prev = _vuln('prev', status='ACKNOWLEDGED')
new = _vuln('new', status='FIXED')
updates = compute_duplicate_updates([prev], [new], copy_fields=['status'])
assert 'status' not in updates['new']

def test_prior_new_status_not_carried(self):
"""A prior status of NEW is treated as unset and is not carried forward."""
prev = _vuln('prev', status='NEW')
new = _vuln('new', status='NEW')
updates = compute_duplicate_updates([prev], [new], copy_fields=['status'])
assert 'status' not in updates['new']

def test_non_status_field_keeps_not_value_semantics(self):
"""Generic fields still use the `not value` emptiness check."""
# Prior verified=True copies onto new verified=False (falsy -> empty).
prev = _vuln('prev', verified=True)
new = _vuln('new', verified=False)
updates = compute_duplicate_updates([prev], [new], copy_fields=['verified'])
assert updates['new']['verified'] is True

# Prior verified=False is empty -> nothing to copy.
prev2 = _vuln('prev2', verified=False)
new2 = _vuln('new2', verified=True)
updates2 = compute_duplicate_updates([prev2], [new2], copy_fields=['verified'])
assert 'verified' not in updates2['new2']


if __name__ == '__main__':
unittest.main()
26 changes: 26 additions & 0 deletions tests/unit/test_output_types.py
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,32 @@ def test_merge_with_exclude_fields(self):
assert vuln1.name == 'CVE-2025-53020'


class TestVulnerabilityStatus(unittest.TestCase):

def test_status_defaults_to_new(self):
vuln = Vulnerability(name='CVE-2025-53020')
assert vuln.status == 'NEW'

def test_status_empty_coerces_to_new(self):
assert Vulnerability(name='CVE-2025-53020', status='').status == 'NEW'
assert Vulnerability(name='CVE-2025-53020', status=None).status == 'NEW'

def test_status_unknown_coerces_to_new(self):
assert Vulnerability(name='CVE-2025-53020', status='bogus').status == 'NEW'

def test_status_valid_values_preserved_and_uppercased(self):
assert Vulnerability(name='CVE-2025-53020', status='ACKNOWLEDGED').status == 'ACKNOWLEDGED'
assert Vulnerability(name='CVE-2025-53020', status='fixed').status == 'FIXED'
assert Vulnerability(name='CVE-2025-53020', status=' new ').status == 'NEW'

def test_status_does_not_affect_equality(self):
# Same identity (name/id/matched_at) but different status must still be equal (dedup-safe).
vuln1 = Vulnerability(name='CVE-2025-53020', id='CVE-2025-53020', matched_at='host:80', status='NEW')
vuln2 = Vulnerability(name='CVE-2025-53020', id='CVE-2025-53020', matched_at='host:80', status='FIXED')
assert vuln1 == vuln2
assert vuln1._compare_key() == vuln2._compare_key()


class TestErrorRich(unittest.TestCase):

def test_error_rich_with_node_id(self):
Expand Down
Loading