diff --git a/src/storage/db_interface_frontend.py b/src/storage/db_interface_frontend.py index c10c6a919..276548690 100644 --- a/src/storage/db_interface_frontend.py +++ b/src/storage/db_interface_frontend.py @@ -173,6 +173,18 @@ def get_latest_comments(self, limit=10): query = select(subquery).order_by(subquery.c.jsonb_array_elements.cast(JSONB)['time'].desc()) return [{'uid': uid, **comment_dict} for uid, comment_dict in session.execute(query.limit(limit))] + def get_comments_for_firmware(self, fw_uid: str) -> dict[str, list[dict]]: + if not self.is_firmware(fw_uid): + return {} + with self.get_read_only_session() as session: + query = ( + select(FileObjectEntry.comments, FileObjectEntry.uid) + .join(fw_files_table, fw_files_table.c.file_uid == FileObjectEntry.uid) + .filter(fw_files_table.c.root_uid == fw_uid) + .filter(FileObjectEntry.comments != []) + ) + return {uid: comment_list for comment_list, uid in session.execute(query) if uid != fw_uid} + # --- generic search --- def generic_search( diff --git a/src/test/common_helper.py b/src/test/common_helper.py index 400f51adb..593923335 100644 --- a/src/test/common_helper.py +++ b/src/test/common_helper.py @@ -264,6 +264,10 @@ def get_file_tree_path(uid: str, root_uid=None): return [[root_uid, uid]] return [[uid]] + @staticmethod + def get_comments_for_firmware(uid: str): # noqa: ARG004 + return {} + def fake_exit(self, *args): # noqa: ARG001 pass diff --git a/src/test/integration/storage/test_db_interface_frontend.py b/src/test/integration/storage/test_db_interface_frontend.py index 57df5dae9..4f32c617a 100644 --- a/src/test/integration/storage/test_db_interface_frontend.py +++ b/src/test/integration/storage/test_db_interface_frontend.py @@ -433,6 +433,24 @@ def test_get_latest_comments(frontend_db, backend_db): assert result[1]['uid'] == 'fo2_uid' +def test_get_comments_for_firmware(frontend_db, backend_db): + fw, parent_fo, child_fo = create_fw_with_parent_and_child() + backend_db.insert_object(fw) + assert frontend_db.get_comments_for_firmware(fw.uid) == {} + parent_fo.comments = [{'author': 'anonymous', 'comment': 'comment1', 'time': '1'}] + child_fo.comments = [ + {'author': 'anonymous', 'comment': 'comment2', 'time': '2'}, + {'author': 'anonymous', 'comment': 'comment3', 'time': '3'}, + ] + backend_db.insert_multiple_objects(parent_fo, child_fo) + agg_comments = frontend_db.get_comments_for_firmware(fw.uid) + assert len(agg_comments) == 2 + assert parent_fo.uid in agg_comments + assert child_fo.uid in agg_comments + assert len(agg_comments[child_fo.uid]) == 2 + assert agg_comments[parent_fo.uid][0]['comment'] == 'comment1' + + def test_generate_file_tree_level(frontend_db, backend_db): child_fo, parent_fw = create_fw_with_child_fo() child_fo.processed_analysis['file_type'] = generate_analysis_entry(analysis_result={'mime': 'sometype'}) diff --git a/src/test/unit/web_interface/test_app_show_analysis.py b/src/test/unit/web_interface/test_app_show_analysis.py index 8f2c7b4ce..24622667f 100644 --- a/src/test/unit/web_interface/test_app_show_analysis.py +++ b/src/test/unit/web_interface/test_app_show_analysis.py @@ -93,3 +93,21 @@ def test_app_failed_analysis(self, test_client): assert 'Failed' in template assert 'reason for fail' in template assert 'class="table-danger"' in template, 'failed result should be rendered in "danger" style' + + +class AggCommentDbMock(DbMock): + def get_comments_for_firmware(self, uid): + if uid == TEST_FW.uid: + return { + 'included_file_uid': [ + {'time': 0, 'author': 'some_guy', 'plugin': 'plugin_name'}, + ] + } + return {} + + +@pytest.mark.WebInterfaceUnitTestConfig(intercom_mock_class=IntercomMock, database_mock_class=AggCommentDbMock) +def test_show_aggregated_comments(test_client): + response = test_client.get(f'/analysis/{TEST_FW.uid}').data.decode() + assert 'Comments for Included Files' in response + assert 'some_guy' in response diff --git a/src/web_interface/components/analysis_routes.py b/src/web_interface/components/analysis_routes.py index 308ea9341..cfc97a945 100644 --- a/src/web_interface/components/analysis_routes.py +++ b/src/web_interface/components/analysis_routes.py @@ -51,26 +51,26 @@ def __init__(self, *args, **kwargs): @AppRoute('/analysis//', GET) @AppRoute('/analysis///ro/', GET) def show_analysis(self, uid, selected_analysis=None, root_uid=None): - other_versions = None - all_comparisons = self.db.comparison.page_comparison_results() with get_shared_session(self.db.frontend) as frontend_db: - known_comparisons = [comparison for comparison in all_comparisons if uid in comparison[0]] file_obj = frontend_db.get_object(uid) if not file_obj: return render_template('uid_not_found.html', uid=uid) if selected_analysis is not None and selected_analysis not in file_obj.processed_analysis: flash(f'The requested analysis ({selected_analysis}) has not run (yet)', 'warning') selected_analysis = None + if isinstance(file_obj, Firmware): root_uid = file_obj.uid other_versions = frontend_db.get_other_versions_of_firmware(file_obj) + file_tree_paths = [[file_obj.uid]] + aggregated_comments = frontend_db.get_comments_for_firmware(file_obj.uid) + else: # FileObject (i.e. not the root Firmware object) + other_versions = None + file_tree_paths = frontend_db.get_file_tree_path(uid, root_uid=none_to_none(root_uid)) + aggregated_comments = {} + included_fo_analysis_complete = not frontend_db.all_uids_found_in_database(list(file_obj.files_included)) - file_tree_paths = ( - frontend_db.get_file_tree_path(uid, root_uid=none_to_none(root_uid)) - if not isinstance(file_obj, Firmware) - else [[file_obj.uid]] - ) - analysis_plugins = self.intercom.get_available_analysis_plugins() + analysis_plugins = self.intercom.get_available_analysis_plugins() analysis = file_obj.processed_analysis.get(selected_analysis, {}) @@ -88,11 +88,14 @@ def show_analysis(self, uid, selected_analysis=None, root_uid=None): other_versions=other_versions, uids_for_comparison=get_comparison_uid_dict_from_session(), user_has_admin_clearance=user_has_privilege(current_user, privilege='delete'), - known_comparisons=known_comparisons, + known_comparisons=[ + comparison for comparison in self.db.comparison.page_comparison_results() if uid in comparison[0] + ], available_plugins=self._get_used_and_unused_plugins( file_obj.processed_analysis, [x for x in analysis_plugins if x != 'unpacker'] ), link_target=self._get_link_target(file_obj, root_uid), + aggregated_comments=aggregated_comments, ) def _get_correct_template(self, selected_analysis: str | None, fw_object: Firmware | FileObject): @@ -212,8 +215,7 @@ def show_elf_dependency_graph(self, uid: str, root_uid: str): colors = sorted(get_graph_colors(len(data_graph_part['groups']))) if not data_graph_part['nodes']: flash( - 'Error: Graph could not be rendered. ' - 'The file chosen as root must contain a filesystem with binaries.', + 'Error: Graph could not be rendered. The file chosen as root must contain a filesystem with binaries.', 'danger', ) return render_template('dependency_graph.html', **data_graph_part, uid=uid, root_uid=root_uid) diff --git a/src/web_interface/templates/generic_view/tags.html b/src/web_interface/templates/generic_view/tags.html index 652ef05d7..5c9ca9bfc 100644 --- a/src/web_interface/templates/generic_view/tags.html +++ b/src/web_interface/templates/generic_view/tags.html @@ -7,6 +7,8 @@ {%- endif %} {% if uid -%} onclick="location.href='/analysis/{{ uid }}/{{ plugin }}/ro/{{ root_uid }}?load_summary=true'" + {% elif value == "Comments" %} + onclick="document.getElementById('comments-head').scrollIntoView({ behavior: 'smooth', block: 'start' });" {%- endif %} > {{ value }} diff --git a/src/web_interface/templates/show_analysis.html b/src/web_interface/templates/show_analysis.html index efcc47927..d1faccf01 100644 --- a/src/web_interface/templates/show_analysis.html +++ b/src/web_interface/templates/show_analysis.html @@ -70,10 +70,14 @@

{% if link_target %} ({{ link_target | safe }}) {% endif %} -
+
{% if firmware.analysis_tags or firmware.tags %} - {{ firmware.analysis_tags | render_analysis_tags(uid, root_uid) | safe }}{{ firmware.tags | render_fw_tags | safe }}
+ {{ firmware.analysis_tags | render_analysis_tags(uid, root_uid) | safe }}{{ firmware.tags | render_fw_tags | safe }} {% endif %} + {% if firmware.comments %} + {{ {"Comments": "secondary"} | render_fw_tags | safe }} + {% endif %} +
UID: {{ uid | safe }}

{% if all_analyzed_flag %} diff --git a/src/web_interface/templates/show_analysis/comments.j2 b/src/web_interface/templates/show_analysis/comments.j2 index f6fa76ac8..794eeeb32 100644 --- a/src/web_interface/templates/show_analysis/comments.j2 +++ b/src/web_interface/templates/show_analysis/comments.j2 @@ -1,3 +1,38 @@ +{% macro render_comment(comment, delete=True) %} + + + {{ comment.plugin or "—" }} +
+ {{ comment.author }} +
+ {{ comment.time | int | nice_unix_time }} + + + + {{ comment.comment | urlize }} + + + {% if delete %} + + {# Comment Delete Button #} + + + + {# Comment Delete Confirm Button #} +
+ +
+ + {% endif %} + +{% endmacro %} +
@@ -81,40 +116,38 @@ {% for comment in firmware.comments | sort_comments %} - - - - - - + {{ render_comment(comment, True) }} {% endfor %}
- {{ comment.plugin or "—" }} -
- {{ comment.author }} -
- {{ comment.time | int | nice_unix_time }} - 		- - {{ comment.comment | urlize }} - - - {# Comment Delete Button #} - - - {# Comment Delete Confirm Button #} -
- -
-
{% endif %}
+ {% if aggregated_comments %} +
+ + + + + + + + {% for uid, comment_list in aggregated_comments.items() %} + + + + + {% endfor %} + +
Comments for Included Files
+ {{ uid | replace_uid_with_hid_link }} + + + {% for comment in comment_list | sort_comments %} + {{ render_comment(comment, False) }} + {% endfor %} +
+
+
+ {% endif %}