Resolve concatenation in the html-safety check, and empty the baseline

admin/scripts/check_html_safety.py

@@ -45,7 +45,7 @@
     A literal remote URL appears in a markup-bearing string, or an `href=`/`src=`
     attribute is completed by an interpolation. A dynamic destination cannot be shown
     to be report-relative by reading the source, so it fails unless it comes from
-    `safe_local_link()`.
+    `safe_local_path()` or `safe_local_link()`.
 
 `unguarded-html-columns`
     A module declares `html_columns` but never references an escaper. This catches the
@@ -125,16 +125,9 @@
 
 # Pre-existing violations. Delete an entry when its violation is fixed; a stale entry
 # fails the run. See the module docstring before adding one.
-BASELINE = {
-    # media_to_html() assigns `source` four times before emitting it -- the raw match,
-    # a relative path, a copied path, then safe_local_path(). A name is treated as safe
-    # only when *every* assignment to it is, because this check does not order
-    # assignments. The function is in fact correct: the last write is safe_local_path()
-    # and nothing reads `source` before it. Rewriting it to bind the escaped value to
-    # its own name would clear this honestly.
-    ('scripts/ilapfuncs.py', 'remote-destination', 'media_to_html'),
-    ('scripts/ilapfuncs.py', 'unescaped-interpolation', 'media_to_html'),
-}
+#
+# Empty: every finding this core had is now fixed rather than carried.
+BASELINE = set()
 
 # Reviewed exceptions expected to stay. Every entry needs a comment saying why.
 ALLOWLIST = set()
@@ -276,6 +269,12 @@ def _resolve(node, assignments, names, seen):
     if isinstance(node, ast.JoinedStr):
         return all(_resolve(v.value, assignments, names, seen)
                    for v in node.values if isinstance(v, ast.FormattedValue))
+    # `agg = agg + f'<td>{esc(v)}</td>'` -- the accumulator shape most of these
+    # builders use. A concatenation is safe when both sides are, and the
+    # self-reference on the left terminates through `seen`.
+    if isinstance(node, ast.BinOp) and isinstance(node.op, ast.Add):
+        return (_resolve(node.left, assignments, names, seen)
+                and _resolve(node.right, assignments, names, seen))
     if isinstance(node, ast.Call):
         if call_name(node) in names:
             return True

scripts/ilapfuncs.py

@@ -928,17 +928,21 @@ def relative_paths(source, splitter):
         # allow_parent: relative_paths() above deliberately emits ../data/... to reach
         # the extraction folder beside the report. The evidence filename in the
         # fallback link text is escaped -- it used to be interpolated raw.
-        source = safe_local_path(source, allow_parent=True)
-        filename = esc(filename)
+        # Bind the escaped values to their own names rather than writing back over
+        # `source`, which is assigned several times above. Reading a name that only
+        # ever holds a checked value makes the safety local and obvious, to a reader
+        # and to admin/scripts/check_html_safety.py alike.
+        safe_source = safe_local_path(source, allow_parent=True)
+        safe_filename = esc(filename)
 
         if 'video' in mimetype:
-            thumb = f'<video width="320" height="240" controls="controls"><source src="{source}" type="video/mp4" preload="none">Your browser does not support the video tag.</video>'
+            thumb = f'<video width="320" height="240" controls="controls"><source src="{safe_source}" type="video/mp4" preload="none">Your browser does not support the video tag.</video>'
         elif 'image' in mimetype:
-            thumb = f'<a href="{source}" target="_blank"><img src="{source}" width="300"></img></a>'
+            thumb = f'<a href="{safe_source}" target="_blank"><img src="{safe_source}" width="300"></img></a>'
         elif 'audio' in mimetype:
-            thumb = f'<audio controls><source src="{source}" type="audio/ogg"><source src="{source}" type="audio/mpeg">Your browser does not support the audio element.</audio>'
+            thumb = f'<audio controls><source src="{safe_source}" type="audio/ogg"><source src="{safe_source}" type="audio/mpeg">Your browser does not support the audio element.</audio>'
         else:
-            thumb = f'<a href="{source}" target="_blank"> Link to {filename} file</a>'
+            thumb = f'<a href="{safe_source}" target="_blank"> Link to {safe_filename} file</a>'
     return thumb