Skip to content

Commit b6ee9ca

Browse files
authored
fix: prevent operation report from returning null when link paw absent (#3048) (#3279)
* fix: prevent operation report from returning null when a link paw is absent (#3048) Three related KeyErrors in c_operation.py could cause Operation.report() to silently return None, which the API then serialised as JSON null and the UI rendered as "Null": 1. `agents_steps[step.paw]` in report() raised KeyError when a link's paw was not in the set of operation agents built at call time (e.g. the agent was removed between operation run and report download). Fixed with `agents_steps.setdefault(step.paw, {'steps': []})`. 2. `abilities_by_agent[link.paw]` in _get_all_possible_abilities_by_agent() had the same pattern — orphan paw not guarded. Fixed with an explicit membership check before the extend. 3. The `except Exception` block in report() logged the error but fell off the end of the function, returning None implicitly. The caller then returned None to web.json_response(), producing the "Null" download. Fixed by re-raising so the framework returns a proper 500 with an error body instead of a silent null payload. Adds a regression test that constructs an operation with a chain link whose paw is not present in operation.agents and asserts report() returns a non-None dict that includes the orphan paw's steps. * style: fix E303 too many blank lines in test_operation.py * refactor: simplify double dict lookup using abilities_by_agent.get()
1 parent 981ab92 commit b6ee9ca

File tree

2 files changed

+47
-3
lines changed

2 files changed

+47
-3
lines changed

app/objects/c_operation.py

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -338,12 +338,13 @@ async def report(self, file_svc, data_svc, output=False):
338338
step_report['output'] = json.loads(results.replace('\\r\\n', '').replace('\\n', ''))
339339
if step.agent_reported_time:
340340
step_report['agent_reported_time'] = step.agent_reported_time.strftime(self.TIME_FORMAT)
341-
agents_steps[step.paw]['steps'].append(step_report)
341+
agents_steps.setdefault(step.paw, {'steps': []})['steps'].append(step_report)
342342
report['steps'] = agents_steps
343343
report['skipped_abilities'] = await self.get_skipped_abilities_by_agent(data_svc)
344344
return report
345345
except Exception:
346-
logging.error('Error saving operation report (%s)' % self.name, exc_info=True)
346+
logging.error('Error generating operation report (%s)' % self.name, exc_info=True)
347+
raise
347348

348349
async def event_logs(self, file_svc, data_svc, output=False):
349350
# Ignore discarded / high visibility links that did not actually run.
@@ -487,7 +488,9 @@ async def _get_all_possible_abilities_by_agent(self, data_svc):
487488
for link in self.chain:
488489
if link.ability.ability_id not in self.adversary.atomic_ordering:
489490
matching_abilities = await data_svc.locate('abilities', match=dict(ability_id=link.ability.ability_id))
490-
abilities_by_agent[link.paw]['all_abilities'].extend(matching_abilities)
491+
entry = abilities_by_agent.get(link.paw)
492+
if entry:
493+
entry['all_abilities'].extend(matching_abilities)
491494
return abilities_by_agent
492495

493496
def _check_reason_skipped(self, agent, ability, op_facts, state, agent_executors, agent_ran):

tests/objects/test_operation.py

Lines changed: 41 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -627,6 +627,47 @@ async def test_add_ignored_link(self, make_test_link, operation_agent):
627627
assert test_link.id in op.ignored_links
628628
assert len(op.ignored_links) == 1
629629

630+
async def test_report_includes_steps_for_agents_not_in_host_group(
631+
self, operation_agent, operation_adversary, executor, ability, operation_link,
632+
encoded_command, parse_datestring, file_svc, data_svc, knowledge_svc, fire_event_mock):
633+
"""Regression test for issue #3048: a link whose paw is absent from operation.agents
634+
must not cause report() to silently return None (i.e. download as 'Null')."""
635+
from app.objects.c_planner import Planner
636+
from app.objects.c_objective import Objective
637+
638+
op = Operation(name='report-test', agents=[operation_agent], adversary=operation_adversary)
639+
op.set_start_details()
640+
op.planner = Planner(planner_id='testplanner', name='test_planner', module='test', params=None)
641+
op.objective = Objective(id='obj1', name='test objective')
642+
643+
exe = executor(name='psh', platform='windows', command='whoami')
644+
ab = ability(ability_id='rep123', tactic='test tactic', technique_id='T0000',
645+
technique_name='test technique', name='test ability',
646+
description='test desc', executors=[exe])
647+
648+
known_link = operation_link(
649+
command=encoded_command('whoami'),
650+
plaintext_command=encoded_command('whoami'),
651+
paw=operation_agent.paw,
652+
ability=ab, executor=exe, status=0, host=operation_agent.host, pid=1,
653+
decide=parse_datestring(LINK1_DECIDE_TIME),
654+
)
655+
orphan_paw = 'orphan-paw-not-in-agents'
656+
orphan_link = operation_link(
657+
command=encoded_command('id'),
658+
plaintext_command=encoded_command('id'),
659+
paw=orphan_paw,
660+
ability=ab, executor=exe, status=0, host='orphan-host', pid=2,
661+
decide=parse_datestring(LINK2_DECIDE_TIME),
662+
)
663+
op.chain = [known_link, orphan_link]
664+
665+
report = await op.report(file_svc, data_svc, output=False)
666+
assert report is not None, 'report() must not return None when a link paw is absent from agents'
667+
assert 'steps' in report
668+
assert orphan_paw in report['steps'], 'orphan paw steps must appear in report'
669+
assert operation_agent.paw in report['steps'], 'known agent paw steps must appear in report'
670+
630671
async def test_operation_cleanup_status(self, fake_planning_svc, operation_agent):
631672
services = {'planning_svc': fake_planning_svc}
632673
op = Operation(name='test with cleanup', agents=[operation_agent], state='running')

0 commit comments

Comments
 (0)