From ec82de7984feba37a7e958df0b4c84877383a59c Mon Sep 17 00:00:00 2001 From: Brigs Date: Thu, 6 Aug 2026 23:20:57 -0400 Subject: [PATCH] Treat an unreadable __artifacts_v2__ as a sink, not as a non-sink A module whose __artifacts_v2__ is built by a helper cannot be evaluated statically, so declares_html_columns() returns None. is_sink read that as falsy, which meant the module was skipped by all three rules -- not just by unguarded-html-columns, which is the only one that actually needs the declaration. That is fail-open in the one case where the answer is unknown. A module in the NOT CHECKED list could gain an html_columns tomorrow and stay completely unguarded, with nothing in the output changing to say so. Now an unreadable declaration is treated as a sink. The uncertainty costs coverage of one rule instead of all three. Verified against a probe artifact whose __artifacts_v2__ is helper-built and which carries both an unescaped interpolation and a remote : the old behaviour reported 0 findings for it, this reports 3. Measured before committing: no module currently in any core's NOT CHECKED list produces a finding under the new default, so this costs nothing today. It was checked the other way too -- of the eleven such modules across the cores, ten declare no html_columns at runtime and the eleventh (RLEAPP kikReturns) was already being scanned through a media helper. The hole was latent, not live. The NOT CHECKED banner now says the other two rules still run, so the list reads as one missing rule rather than as an unscanned module. Validated: pylint --disable=C,R = 10.00; lint_changed reports no new warnings; unit tests OK; the check still exits 0 on this tree. Co-Authored-By: Claude Opus 5 --- admin/scripts/check_html_safety.py | 15 ++++++++++++--- 1 file changed, 12 insertions(+), 3 deletions(-) diff --git a/admin/scripts/check_html_safety.py b/admin/scripts/check_html_safety.py index 57023b3..8209d67 100644 --- a/admin/scripts/check_html_safety.py +++ b/admin/scripts/check_html_safety.py @@ -68,7 +68,9 @@ Coverage holes are printed, never hidden. A module whose `__artifacts_v2__` is not a static literal cannot have its `html_columns` read, so `unguarded-html-columns` cannot -run on it; those are listed as NOT CHECKED on every run. +run on it; those are listed as NOT CHECKED on every run. The other two rules do still +run on them -- an unreadable declaration is treated as a sink rather than as a +non-sink, so the uncertainty costs coverage of one rule instead of all three. `scripts/artifact_report.py` is deliberately out of scope. It implements the escape/no-escape branch itself, so it is the sink these rules protect, not a producer. @@ -444,7 +446,13 @@ def scan_file(path, rel_path): # Elsewhere the writer escapes the whole cell, so a hand-built tag renders as # visible text -- a display bug, not an injection, and not this check's business. in_scope_functions = FRAMEWORK_FUNCTIONS.get(rel_path) - is_sink = bool(declares) or uses_media_helper(tree) or in_scope_functions + # `declares` is None when __artifacts_v2__ could not be read statically. Treat + # that as a sink: an unreadable declaration is the one case where we cannot tell, + # and guessing "not a sink" turns all three rules off silently. A module can then + # gain an html_columns and stay unguarded with nothing in the output changing. + # Assume the worst when the answer is unknown. + is_sink = (declares is None or bool(declares) or uses_media_helper(tree) + or in_scope_functions) findings = [] if is_sink: @@ -515,7 +523,8 @@ def main(): if skipped: print(f'NOT CHECKED -- {len(skipped)} module(s) have no statically readable ' - f'__artifacts_v2__, so html_columns could not be read:') + f'__artifacts_v2__, so html_columns could not be read. They are still ' + f'scanned for unescaped markup and remote destinations:') for rel_path, reason in skipped: print(f' {rel_path}: {reason}') print()