fix(report): link advisories in the by-device table and stop it reflowing (4.2.1)

VERSION

@@ -1 +1 @@
-4.2.0
+4.2.1

caia/bundle_report.py

@@ -240,9 +240,11 @@ def _undetermined_appendix(devices):
         '<p>Each appears above as <em>undetermined</em> against every advisory that covers '
         'its product family, and nowhere as unaffected. Correcting the inventory values '
         'below is what makes the next bundle review conclusive.</p>'
-        '<table class="plain"><thead><tr><th>Device</th><th>Model recorded</th>'
-        '<th>Release recorded</th><th>Why</th></tr></thead>'
-        f'<tbody>{rows}</tbody></table></section>')
+        '<div class="tscroll"><table class="plain table-fixed"><thead><tr>'
+        '<th class="col-dev">Device</th><th class="col-model">Model recorded</th>'
+        '<th class="col-rel">Release recorded</th><th class="col-why">Why</th>'
+        "</tr></thead>"
+        f'<tbody>{rows}</tbody></table></div></section>')
 
 
 def _scope_appendix(bundle):
@@ -255,29 +257,45 @@ def _scope_appendix(bundle):
 
 
 def _device_index(rows, devices):
-    """Per-device cross-reference: one device to every bundle advisory affecting it (FR-036)."""
+    """Per-device cross-reference: one device to every bundle advisory affecting it (FR-036).
+
+    The advisory identifiers link to Cisco, matching the per-advisory rows above — a reader
+    working device by device should not have to scroll back up to reach the advisory.
+
+    Column widths are fixed (`table-fixed`) so expanding a device's disclosure does not
+    reflow the table: the rows above and below the one being read would otherwise shift
+    under the cursor, which is exactly when a reader is trying to compare them.
+    """
     by_device = {}
     for row in rows:
         for device in row.impacted:
-            by_device.setdefault(id(device), (device, []))[1].append(row.advisory_id)
+            entry = by_device.setdefault(id(device), (device, []))
+            entry[1].append((row.advisory_id, row.url))
     if not by_device:
         return ""
     entries = []
-    for device, advisory_ids in sorted(by_device.values(),
-                                       key=lambda pair: str(pair[0].get("name") or "")):
-        chips = "".join(f'<li>{esc(a)}</li>' for a in sorted(advisory_ids))
+    for device, advisories in sorted(by_device.values(),
+                                     key=lambda pair: str(pair[0].get("name") or "")):
+        links = []
+        for advisory_id, url in sorted(advisories):
+            label = esc(advisory_id)
+            links.append(
+                f'<li><a href="{esc(url)}" target="_blank" rel="noreferrer noopener">'
+                f'{label}</a></li>' if url else f"<li>{label}</li>")
         entries.append(
             f'<tr><td>{esc(device.get("name"))}</td>'
             f'<td>{esc(device.get("model"))}</td><td>{esc(device.get("version"))}</td>'
-            f'<td>{len(advisory_ids)}</td>'
-            f'<td><details><summary>show</summary><ul class="idlist">{chips}</ul>'
+            f'<td>{len(advisories)}</td>'
+            f'<td><details><summary>show</summary><ul class="idlist">{"".join(links)}</ul>'
             "</details></td></tr>")
     return ('<section class="appendix"><h2>By device</h2>'
             '<p>The same findings, inverted, for working device by device. Devices that '
             'could not be assessed are listed separately above.</p>'
-            '<table class="plain"><thead><tr><th>Device</th><th>Model</th><th>Release</th>'
-            '<th>Advisories</th><th>Which</th></tr></thead>'
-            f'<tbody>{"".join(entries)}</tbody></table></section>')
+            '<div class="tscroll"><table class="plain table-fixed"><thead><tr>'
+            '<th class="col-dev">Device</th><th class="col-model">Model</th>'
+            '<th class="col-rel">Release</th><th class="col-count">Advisories</th>'
+            '<th class="col-which">Which</th></tr></thead>'
+            f'<tbody>{"".join(entries)}</tbody></table></div></section>')
 
 
 def _controls(rows):
@@ -316,6 +334,27 @@ def _controls(rows):
 th { background: #1f4e78; color: #fff; font-weight: 600; position: sticky; top: 0; }
 tbody tr:nth-child(even) { background: #fbfbfc; }
 table.plain { box-shadow: none; border: 1px solid #e2e4e7; }
+/* Fixed layout so expanding a row's disclosure cannot reflow the columns. Without it the
+   rows a reader is comparing shift sideways at the moment they open one.
+   `min-width` keeps the columns usable on a narrow window: the table scrolls horizontally
+   inside .tscroll rather than squeezing the advisory column until identifiers break across
+   four lines each. */
+/* 900px is not arbitrary: the longest advisory identifier measures 316px in this monospace
+   at this size, and 40% of 900 leaves 360px — enough for one line plus the list indent.
+   Narrower and every identifier wraps across three or four lines. */
+table.table-fixed { table-layout: fixed; width: 100%; min-width: 900px; }
+.tscroll { overflow-x: auto; }
+table.table-fixed th, table.table-fixed td { overflow-wrap: anywhere; }
+/* Identifiers are meant to be read and copied whole, so they wrap between words only —
+   never mid-token like the prose cells above. */
+table.table-fixed .idlist a { overflow-wrap: normal; word-break: keep-all; }
+/* Both fixed tables share the first three columns; each set sums to 100%. */
+.col-dev   { width: 16%; }
+.col-model { width: 23%; }
+.col-rel   { width: 12%; }
+.col-count { width: 9%; }
+.col-which { width: 40%; }
+.col-why   { width: 49%; }
 .c-adv { width: 30%; } .c-date { width: 12%; white-space: nowrap; }
 .advid { font-weight: 600; margin-top: .3rem; font-family: ui-monospace, SFMono-Regular,
          Menlo, monospace; font-size: .85rem; }

tests/test_bundle_report.py

@@ -270,6 +270,102 @@ def test_per_device_cross_reference_is_present(self):
         self.assertIn("fmc-0", html)
 
 
+class DeviceIndexTable(unittest.TestCase):
+    """The 'By device' appendix — links, and stable geometry while disclosing."""
+
+    def _index_section(self, html):
+        start = html.index("<h2>By device</h2>")
+        return html[start:html.index("</section>", start)]
+
+    def test_advisory_ids_link_to_cisco_in_a_new_tab(self):
+        section = self._index_section(rendered()[0])
+        links = re.findall(r'<li><a href="([^"]+)"([^>]*)>(cisco-sa-[^<]+)</a></li>',
+                           section)
+        self.assertTrue(links, "advisory identifiers must be links")
+        for url, attrs, label in links:
+            self.assertIn("cisco.com", url)
+            self.assertIn(label, url)
+            self.assertIn('target="_blank"', attrs)
+            self.assertIn("noopener", attrs)
+
+    def test_every_advisory_for_a_device_is_linked(self):
+        section = self._index_section(rendered()[0])
+        listed = re.findall(r"<li>(?:<a[^>]*>)?(cisco-sa-[A-Za-z0-9._-]+)", section)
+        linked = re.findall(r"<li><a[^>]*>(cisco-sa-[A-Za-z0-9._-]+)</a>", section)
+        self.assertEqual(sorted(listed), sorted(linked),
+                         "an unlinked identifier means a reader has to scroll back up")
+
+    def test_an_advisory_without_a_url_still_renders(self):
+        """A row whose facts could not be fetched must not lose its identifier."""
+        bare = advisory(FMC_ADV)
+        bare["publicationUrl"] = ""
+        devices = [device("a", "FMC", advisories=[bare])]
+        html, _ = rendered(devices=devices,
+                           bundle=a_bundle(advisory_ids=(FMC_ADV,)), records={})
+        section = self._index_section(html)
+        self.assertIn(FMC_ADV, section)
+        self.assertNotIn('href=""', section)
+
+    def test_columns_do_not_reflow_when_a_disclosure_opens(self):
+        """Fixed layout: expanding one row must not shift the rows being compared."""
+        html, _ = rendered()
+        section = self._index_section(html)
+        self.assertIn('class="plain table-fixed"', section)
+        for css_class in ("col-dev", "col-model", "col-rel", "col-count", "col-which"):
+            self.assertIn(css_class, section)
+        self.assertIn("table-layout: fixed", html)
+
+    def test_every_fixed_column_has_a_declared_width(self):
+        html, _ = rendered()
+        for css_class in ("col-dev", "col-model", "col-rel", "col-count",
+                          "col-which", "col-why"):
+            self.assertRegex(html, rf"\.{css_class}\s*\{{[^}}]*width:")
+
+    def test_undetermined_table_is_also_fixed(self):
+        """Same disclosure-reflow problem, same fix."""
+        html, _ = rendered()
+        start = html.index("could not be assessed")
+        section = html[start:html.index("</section>", start)]
+        self.assertIn("table-fixed", section)
+        self.assertIn("col-why", section)
+
+    def test_long_values_wrap_rather_than_widening_a_column(self):
+        html, _ = rendered()
+        self.assertIn("overflow-wrap: anywhere", html)
+
+    def test_identifiers_are_not_broken_mid_token(self):
+        """An identifier split across four lines is unreadable and uncopyable."""
+        html, _ = rendered()
+        self.assertRegex(html, r"\.idlist a\s*\{[^}]*overflow-wrap:\s*normal")
+        self.assertRegex(html, r"\.idlist a\s*\{[^}]*word-break:\s*keep-all")
+
+    def test_fixed_tables_have_a_minimum_width_and_scroll_when_narrower(self):
+        """Fixed columns must not squeeze the advisory column on a narrow window.
+
+        The longest identifier measures ~316px in this monospace at this size, so the
+        advisory column needs ~334px including the list indent. 40% of the 900px floor
+        gives 360px; anything much narrower wraps every identifier.
+        """
+        html, _ = rendered()
+        self.assertRegex(html, r"table\.table-fixed\s*\{[^}]*min-width:\s*900px")
+        self.assertRegex(html, r"\.tscroll\s*\{[^}]*overflow-x:\s*auto")
+        section = self._index_section(html)
+        self.assertIn('<div class="tscroll">', section)
+
+    def test_each_fixed_table_column_set_sums_to_one_hundred_percent(self):
+        """A set summing to anything else makes the declared widths advisory, not fixed."""
+        html, _ = rendered()
+        width = {}
+        for name in ("col-dev", "col-model", "col-rel", "col-count", "col-which", "col-why"):
+            found = re.search(rf"\.{name}\s*\{{[^}}]*width:\s*(\d+)%", html)
+            self.assertIsNotNone(found, f"{name} has no percentage width")
+            width[name] = int(found.group(1))
+        device_index = ("col-dev", "col-model", "col-rel", "col-count", "col-which")
+        undetermined = ("col-dev", "col-model", "col-rel", "col-why")
+        self.assertEqual(sum(width[c] for c in device_index), 100)
+        self.assertEqual(sum(width[c] for c in undetermined), 100)
+
+
 class Output(unittest.TestCase):
     def test_filename_carries_the_bundle_and_keeps_the_prefix(self):
         with tempfile.TemporaryDirectory() as tmp: