Skip to content
Open
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
40 changes: 34 additions & 6 deletions secator/cli.py
Original file line number Diff line number Diff line change
Expand Up @@ -1123,9 +1123,10 @@ def list_aliases(silent):
@click.option('--driver', type=click.Choice(['local', 'mongodb', 'api', 'sqlite']), default=None, help='Query backend driver') # noqa: E501
@click.option('--dedupe/--no-dedupe', default=None, help='Deduplicate findings (defaults to config value)')
@click.option('-l', '--limit', type=int, default=0, help='Limit number of results (0 = no limit)')
@click.option('--group', is_flag=False, flag_value='', default=None, help='Group findings by field(s) (comma-separated) with auto-aggregation. Bare --group uses per-type defaults.') # noqa: E501
@click.option('--save', 'save', type=str, default=None, help='Save the query expression ARG under this name for later reuse (e.g. --save vuln_high)') # noqa: E501
@click.pass_context
def query(ctx, arg, output, output_folder, time_delta, fmt, workspace, report_filter, driver, dedupe, limit, save):
def query(ctx, arg, output, output_folder, time_delta, fmt, workspace, report_filter, driver, dedupe, limit, group, save): # noqa: E501
"""Query"""
# Empty query: return all results (subject to the enforced base query),
# optionally scoped by --report-filter / --workspace.
Expand All @@ -1145,12 +1146,12 @@ def query(ctx, arg, output, output_folder, time_delta, fmt, workspace, report_fi

# 1. Saved query name
if arg in CONFIG.queries:
run_report_show(report_filter, output, time_delta, CONFIG.queries[arg], fmt, workspace, driver, dedupe, limit, output_folder) # noqa: E501
run_report_show(report_filter, output, time_delta, CONFIG.queries[arg], fmt, workspace, driver, dedupe, limit, output_folder, group) # noqa: E501
return

# 2. Raw filter expression
if _looks_like_query_expr(arg):
run_report_show(report_filter, output, time_delta, arg, fmt, workspace, driver, dedupe, limit, output_folder)
run_report_show(report_filter, output, time_delta, arg, fmt, workspace, driver, dedupe, limit, output_folder, group) # noqa: E501
return

# 3. Natural language -> AI chat
Expand Down Expand Up @@ -1363,7 +1364,7 @@ def _apply_format(results, fmt):
return new_results


def run_report_show(report_query, output, time_delta, query, fmt, workspace, driver, dedupe, limit, output_folder=None):
def run_report_show(report_query, output, time_delta, query, fmt, workspace, driver, dedupe, limit, output_folder=None, group=None): # noqa: E501
"""Build and send a consolidated report. Shared by `report show` and `query`.

REPORT_QUERY: comma-separated runner paths (e.g. scans/5,tasks/3).
Expand Down Expand Up @@ -1469,6 +1470,30 @@ def run_report_show(report_query, output, time_delta, query, fmt, workspace, dri
dedupe_effective = CONFIG.runners.remove_duplicates if dedupe is None else dedupe
report = Report(runner, title=f'Consolidated report - {current}', exporters=exporters)
report.build(query=full_query, dedupe=dedupe_effective, limit=limit)

# Group findings by field(s) with auto-aggregation (processing-side, post-query).
# `group is None` => disabled; `group == ''` => per-type defaults; else explicit field(s).
grouped_types = []
if group is not None and not fmt:
from secator.query.utils import group_findings
user_group_by = [f.strip() for f in group.split(',') if f.strip()]
type_map = {cls.get_name(): cls for cls in FINDING_TYPES}
for type_name, items in report.data['results'].items():
cls = type_map.get(type_name)
if not items or cls is None:
continue
group_by = user_group_by or list(getattr(cls, '_group_by', ()) or ())
if not group_by:
continue
aggregate_field = getattr(cls, '_group_aggregate', None)
report.data['results'][type_name] = group_findings(items, group_by, aggregate_field)
grouped_types.append((type_name, ', '.join(group_by)))
if not grouped_types:
group_desc = f' "{group}"' if group else ''
console.print(Warning(message=f'--group{group_desc}: no groupable finding types in results'))
elif group is not None and fmt:
console.print(Warning(message='--group is ignored when --format is used'))

Comment on lines +1473 to +1496

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.

🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win

Bare --group with no groupable types produces no warning.

Line 1477 checks if group and not grouped_types, but when bare --group is used, group == '' (falsy), so the warning never fires. Users who run --group with only non-groupable result types (e.g., URLs, domains) get no feedback that grouping was a no-op.

The PR objectives state grouping should "warn when grouping is unavailable." Fix the condition to also cover the bare-flag case.

🐛 Proposed fix
-		if group and not grouped_types:
-			console.print(Warning(message=f'--group "{group}": no groupable finding types in results'))
+		if not grouped_types:
+			group_desc = f' "{group}"' if group else ''
+			console.print(Warning(message=f'--group{group_desc}: no groupable finding types in results'))
📝 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
# Group findings by field(s) with auto-aggregation (processing-side, post-query).
# `group is None` => disabled; `group == ''` => per-type defaults; else explicit field(s).
grouped_types = []
if group is not None and not fmt:
from secator.query.utils import group_findings
user_group_by = [f.strip() for f in group.split(',') if f.strip()]
type_map = {cls.get_name(): cls for cls in FINDING_TYPES}
for type_name, items in report.data['results'].items():
cls = type_map.get(type_name)
if not items or cls is None:
continue
group_by = user_group_by or list(getattr(cls, '_group_by', ()) or ())
if not group_by:
continue
aggregate_field = getattr(cls, '_group_aggregate', None)
report.data['results'][type_name] = group_findings(items, group_by, aggregate_field)
grouped_types.append((type_name, ', '.join(group_by)))
if group and not grouped_types:
console.print(Warning(message=f'--group "{group}": no groupable finding types in results'))
elif group is not None and fmt:
console.print(Warning(message='--group is ignored when --format is used'))
# Group findings by field(s) with auto-aggregation (processing-side, post-query).
# `group is None` => disabled; `group == ''` => per-type defaults; else explicit field(s).
grouped_types = []
if group is not None and not fmt:
from secator.query.utils import group_findings
user_group_by = [f.strip() for f in group.split(',') if f.strip()]
type_map = {cls.get_name(): cls for cls in FINDING_TYPES}
for type_name, items in report.data['results'].items():
cls = type_map.get(type_name)
if not items or cls is None:
continue
group_by = user_group_by or list(getattr(cls, '_group_by', ()) or ())
if not group_by:
continue
aggregate_field = getattr(cls, '_group_aggregate', None)
report.data['results'][type_name] = group_findings(items, group_by, aggregate_field)
grouped_types.append((type_name, ', '.join(group_by)))
if not grouped_types:
group_desc = f' "{group}"' if group else ''
console.print(Warning(message=f'--group{group_desc}: no groupable finding types in results'))
elif group is not None and fmt:
console.print(Warning(message='--group is ignored when --format is used'))
🤖 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/cli.py` around lines 1459 - 1481, Update the warning condition in the
group-processing block after grouped_types is populated so it triggers when
grouping was requested, including bare --group where group == '', and no
groupable types were found; preserve the existing warning message and avoid
warning when --group was not provided.

if fmt:
report.data['results'] = _apply_format(report.data['results'], fmt)
report.send()
Expand All @@ -1480,6 +1505,8 @@ def run_report_show(report_query, output, time_delta, query, fmt, workspace, dri
if searched:
info_msg += f' (searched: [bold cyan]{searched}[/])'
console.print(Info(message=info_msg))
for type_name, field_str in grouped_types:
console.print(Info(message=f'{type_name} grouped by {field_str}. To show complete results, remove the --group option.')) # noqa: E501


def run_ai_chat(ctx, prompt, workspace):
Expand All @@ -1506,10 +1533,11 @@ def run_ai_chat(ctx, prompt, workspace):
@click.option('--driver', type=click.Choice(['local', 'mongodb', 'api', 'sqlite']), default=None, help='Query backend driver') # noqa: E501
@click.option('--dedupe/--no-dedupe', default=None, help='Deduplicate findings (defaults to config value)')
@click.option('-l', '--limit', type=int, default=0, help='Limit number of results (0 = no limit)')
@click.option('--group', is_flag=False, flag_value='', default=None, help='Group findings by field(s) (comma-separated) with auto-aggregation. Bare --group uses per-type defaults.') # noqa: E501
@click.pass_context
def report_show(ctx, report_query, output, output_folder, time_delta, query, fmt, workspace, driver, dedupe, limit):
def report_show(ctx, report_query, output, output_folder, time_delta, query, fmt, workspace, driver, dedupe, limit, group): # noqa: E501
"""Show report results. REPORT_QUERY: comma-separated runner paths (e.g. scans/5,tasks/3)."""
run_report_show(report_query, output, time_delta, query, fmt, workspace, driver, dedupe, limit, output_folder)
run_report_show(report_query, output, time_delta, query, fmt, workspace, driver, dedupe, limit, output_folder, group)


def _load_report_data(path):
Expand Down
7 changes: 6 additions & 1 deletion secator/exporters/console.py
Original file line number Diff line number Diff line change
Expand Up @@ -31,4 +31,9 @@ def send(self):
item = cls.load(item)
except (TypeError, Exception):
pass
console_stdout.print(item, highlight=False)
# Grouped findings carry a _group_count; show it next to the line.
count = getattr(item, '_group_count', 0)
if count:
console_stdout.print(item, f'[dim](count: {count})[/]', highlight=False)
else:
console_stdout.print(item, highlight=False)
4 changes: 4 additions & 0 deletions secator/output_types/_base.py
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,10 @@
class OutputType:
_table_fields = []
_sort_by = ()
# Fields to group by (tuple) and the field whose values to aggregate/collect
# when the CLI `--group` option is used without an explicit field.
_group_by = ()
_group_aggregate = None

@classmethod
def fields(cls):
Expand Down
2 changes: 2 additions & 0 deletions secator/output_types/tag.py
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,8 @@ class Tag(OutputType):

_table_fields = ['match', 'category', 'name', 'extra_data']
_sort_by = ('match', 'name')
_group_by = ('category', 'name')
_group_aggregate = 'match'

def __post_init__(self):
super().__post_init__()
Expand Down
2 changes: 2 additions & 0 deletions secator/output_types/technology.py
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,8 @@ class Technology(OutputType):

_table_fields = ['match', 'product', 'version', 'extra_data']
_sort_by = ('match', 'product', 'version')
_group_by = ('product',)
_group_aggregate = 'match'

def __str__(self) -> str:
return self.match
Expand Down
2 changes: 2 additions & 0 deletions secator/output_types/vulnerability.py
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,8 @@ class Vulnerability(OutputType):

_table_fields = [MATCHED_AT, SEVERITY, CONFIDENCE, NAME, ID, CVSS_SCORE, STATUS, TAGS, EXTRA_DATA, REFERENCE]
_sort_by = ('confidence_nb', 'severity_nb', 'matched_at', 'cvss_score')
_group_by = (NAME,)
_group_aggregate = MATCHED_AT

@staticmethod
def cvss_to_severity(cvss):
Expand Down
75 changes: 75 additions & 0 deletions secator/query/utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -537,6 +537,81 @@ def emit_query_warnings(warnings):
_warn_unknown_field(field_name, type_name, valid_fields)


def _finding_value(item, key):
"""Read a field from a finding, which may be a dict or an OutputType object."""
return item.get(key) if isinstance(item, dict) else getattr(item, key, None)


def _finding_ts(item):
return _finding_value(item, '_timestamp') or 0


def _truncate_aggregate(values, max_display=5):
"""Join aggregated values, truncating with '.. and X more' past max_display."""
values = [str(v) for v in values]
if len(values) <= max_display:
return ', '.join(values)
shown = ', '.join(values[:max_display])
return f'{shown} .. and {len(values) - max_display} more'


def group_findings(items, group_by, aggregate_field=None, max_display=5):
"""Group findings by one or more fields, collapsing each group to a single
representative (the newest finding) that carries the aggregated field values
and a `_group_count` attribute for display.

Args:
items (list): Findings (dicts or OutputType objects) of a single type.
group_by (list[str]): Field names to group by.
aggregate_field (str): Field whose distinct values are collected onto the
representative (truncated for display). None disables aggregation.
max_display (int): Max aggregated values shown before '.. and X more'.

Returns:
list: One representative finding per group (OutputType objects when the
input could be loaded, else the raw items), in first-seen order.
"""
from secator.output_types import OUTPUT_TYPES
type_map = {cls.get_name(): cls for cls in OUTPUT_TYPES}

groups = {}
order = []
for item in items:
key = tuple(str(_finding_value(item, f)) for f in group_by)
if key not in groups:
groups[key] = {'rep': item, 'agg': [], 'count': 0}
order.append(key)
group = groups[key]
group['count'] += 1
if _finding_ts(item) >= _finding_ts(group['rep']):
group['rep'] = item
if aggregate_field:
value = _finding_value(item, aggregate_field)
if value and value not in group['agg']:
group['agg'].append(value)

out = []
for key in order:
group = groups[key]
rep = group['rep']
if isinstance(rep, dict):
cls = type_map.get(rep.get('_type'))
if cls:
try:
rep = cls.load(rep)
except Exception as e:
debug(f'group_findings: failed to load {rep.get("_type")} representative: {e}', sub='query')
if aggregate_field and not isinstance(rep, dict):
# When grouping, other fields (extra_data, etc.) keep the newest
# finding's values; only the aggregate field is replaced by the
# truncated collected values.
setattr(rep, aggregate_field, _truncate_aggregate(group['agg'], max_display))
if not isinstance(rep, dict):
rep._group_count = group['count']
out.append(rep)
return out


def query_has_type_constraint(query):
"""Check whether a MongoDB-style query contains a '_type' constraint anywhere (recursively).

Expand Down
31 changes: 31 additions & 0 deletions tests/unit/test_query_utils.py
Original file line number Diff line number Diff line change
@@ -1,12 +1,43 @@
from secator.query.utils import (
expand_runner_paths,
group_findings,
parse_report_paths,
python_expr_to_mongo,
validate_query_fields,
query_has_type_constraint,
)


class TestGroupFindings:

def _vuln(self, name, matched_at, ts):
return {'_type': 'vulnerability', 'name': name, 'matched_at': matched_at, '_timestamp': ts}

def test_groups_by_name_with_count_and_aggregate(self):
findings = [
self._vuln('CVE-1', 'http://a.com', ts=1),
self._vuln('CVE-1', 'http://b.com', ts=2),
self._vuln('CVE-2', 'http://c.com', ts=3),
]
grouped = group_findings(findings, group_by=['name'], aggregate_field='matched_at')

assert len(grouped) == 2
by_name = {g.name: g for g in grouped}
assert by_name['CVE-1']._group_count == 2
# Both matched_at values collected into the representative
assert 'http://a.com' in by_name['CVE-1'].matched_at
Comment thread
ocervell marked this conversation as resolved.
Dismissed
assert 'http://b.com' in by_name['CVE-1'].matched_at
Comment thread
ocervell marked this conversation as resolved.
Dismissed
assert by_name['CVE-2']._group_count == 1

def test_aggregate_truncates_past_max_display(self):
findings = [self._vuln('CVE-1', f'http://h{i}.com', ts=i) for i in range(8)]
grouped = group_findings(findings, group_by=['name'], aggregate_field='matched_at', max_display=5)

assert len(grouped) == 1
assert grouped[0]._group_count == 8
assert '.. and 3 more' in grouped[0].matched_at


class TestParseReportPaths:

def test_empty_returns_empty_dict(self):
Expand Down
Loading