fkie-cad · jstucke · May 21, 2025
diff --git 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
@@ -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
@@ -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
@@ -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
@@ -51,26 +51,26 @@ def __init__(self, *args, **kwargs):
     @AppRoute('/analysis/<uid>/<selected_analysis>', GET)
     @AppRoute('/analysis/<uid>/<selected_analysis>/ro/<root_uid>', 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
@@ -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
@@ -70,10 +70,14 @@ <h3>
                 {% if link_target %}
                     ({{ link_target | safe }})
                 {% endif %}
-                <br />
+                <br/>
                 {% if firmware.analysis_tags or firmware.tags %}
-                    {{ firmware.analysis_tags | render_analysis_tags(uid, root_uid) | safe }}{{ firmware.tags | render_fw_tags | safe }}<br />
+                    {{ 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 %}
+                <br/>
                 <span style="font-size: 15px"><strong>UID:</strong> {{ uid | safe }}</span>
             </h3>
             {% if all_analyzed_flag %}

diff --git 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) %}
+    <tr>
+        <td style="vertical-align: bottom; min-width: 205px; width: 20%;">
+            <i class="fas fa-microscope" data-toggle="tooltip" title="analysis plugin"></i> {{ comment.plugin or "—" }}
+            <br>
+            <i class="fas fa-user" data-toggle="tooltip" title="commenter"></i> {{ comment.author }}
+            <br>
+            <i class="far fa-clock"></i> {{ comment.time | int | nice_unix_time }}
+        </td>
+        <td data-toggle="tooltip" title="{{ tooltip }}" style="width: 80%;">
+            <span style="font-family: monospace; font-size: 14px">
+                {{ comment.comment | urlize }}
+            </span>
+        </td>
+        {% if delete %}
+            <td class="m-0 p-0 align-middle" style="width: 10px;">
+                {# Comment Delete Button #}
+                <button data-toggle="collapse" data-target="#delete-button-div-{{ comment.time }}"
+                        class="btn btn-sm">
+                    <i class="fas fa-trash-alt text-danger"></i>
+                </button>
+            </td>
+            <td class="m-0 p-0">
+                {# Comment Delete Confirm Button #}
+                <div class="collapse" id="delete-button-div-{{ comment.time }}">
+                    <button class="btn btn-danger" type="button"
+                            onclick='window.location.href = "/admin/delete_comment/{{ uid }}/{{ comment.time }}";'>
+                        <i class="fas fa-check"></i> Click here to confirm!
+                    </button>
+                </div>
+            </td>
+        {% endif %}
+    </tr>
+{% endmacro %}
+
 <div class="row">
 
     <div class="col-lg-12" style="margin-top: 20px">
@@ -81,40 +116,38 @@
             <table class="table table-bordered p-0 mb-0" id="comments-table">
                 <tbody>
                 {% for comment in firmware.comments | sort_comments %}
-                    <tr>
-                        <td style="vertical-align: bottom; width: 20%;">
-                            <i class="fas fa-microscope" data-toggle="tooltip" title="analysis plugin"></i> {{ comment.plugin or "—" }}
-                            <br>
-                            <i class="fas fa-user" data-toggle="tooltip" title="commenter"></i> {{ comment.author }}
-                            <br>
-                            <i class="far fa-clock"></i> {{ comment.time | int | nice_unix_time }}
-                        </td>
-                        <td data-toggle="tooltip" title="{{ tooltip }}" style="width: 80%;">
-                            <span style="font-family: monospace; font-size: 14px">
-                                {{ comment.comment | urlize }}
-                            </span>
-                        </td>
-                        <td class="m-0 p-0 align-middle">
-                            {# Comment Delete Button #}
-                            <button data-toggle="collapse" data-target="#delete-button-div-{{ comment.time }}"
-                                    class="btn btn-sm">
-                                <i class="fas fa-trash-alt text-danger"></i>
-                            </button>
-                        </td>
-                        <td class="m-0 p-0">
-                            {# Comment Delete Confirm Button #}
-                            <div class="collapse" id="delete-button-div-{{ comment.time }}">
-                                <button class="btn btn-danger" type="button"
-                                        onclick='window.location.href = "/admin/delete_comment/{{ uid }}/{{ comment.time }}";'>
-                                    <i class="fas fa-check"></i> Click here to confirm!
-                                </button>
-                            </div>
-                        </td>
-                    </tr>
+                    {{ render_comment(comment, True) }}
                 {% endfor %}
                 </tbody>
             </table>
         {% endif %}
     </div>
 
+    {% if aggregated_comments %}
+        <div class="col-lg-12" style="margin-top: 20px" id="aggregatedComments">
+            <table class="table table-bordered mb-0" id="comments-head">
+                <thead class="thead-light">
+                    <tr>
+                        <th colspan="2">Comments for Included Files</th>
+                    </tr>
+                </thead>
+                <tbody>
+                    {% for uid, comment_list in aggregated_comments.items() %}
+                        <tr>
+                            <td style="max-width: 200px; word-break: break-all; overflow-wrap: break-word;">
+                                {{ uid | replace_uid_with_hid_link }}
+                            </td>
+                            <td class="p-0">
+                                <table class="table table-bordered mb-0" id="comments-head">
+                                    {% for comment in comment_list | sort_comments %}
+                                        {{ render_comment(comment, False) }}
+                                    {% endfor %}
+                                </table>
+                            </td>
+                        </tr>
+                    {% endfor %}
+                </tbody>
+            </table>
+        </div>
+    {% endif %}
 </div>