fix(gates): harden markup_mask's delimiter scope, then close the string-literal-as-evidence axis in 16 gates (#424) - #438
Merged
Conversation
added 4 commits
August 13, 2026 09:47
…seven private copies (#424) STEP 1 — HARDEN. source_scope treated `<!--` as a comment opener wherever it appeared. `<p>{{ '<!--' }}</p>` … `<img alt="">` … `{{ '-->' }}` blanked the markup between them and gates 35/36 went green over live markup. Replaced the `<!--.*?-->` regex with html_comment_spans(), a scanner that knows a quoted attribute value and a Vue {{ }} interpolation are not comment scope, and that <script>/<style> bodies are raw text. Unterminated openers fall back to 'this was text' rather than swallowing the file — over-blanking is the failure mode. STEP 2 — CONSOLIDATE, only after step 1. Six a11y checkers (gates 37, 39, 40, 42, 43, 44), php_template_scope (gate 41) and check_orphaned_write_capability each carried their own copy of the same regex with the same hole. All eight now call source_scope.mask_html_comments. Also closes the string-literal-as-evidence axis in three PHP gates: gate-8 php_mask(blank_strings=True) — nothing it matches is ever a string, and blanking also repairs the brace walker. gate-59 two masks, one coordinate system: the CALL from a strings-blanked anchor, the config KEY from the string-preserving text. gate-64 same split; strip_comments() (the #184 state machine) now delegates to php_mask, removing a second PHP comment dialect. Every arm labelled EVIDENCE or CONTROL by measurement, not by assertion.
…, 13, 18, 50, 53 (#424) gate-10 / gate-11 TWO MASKS, ONE COORDINATE SYSTEM. Both read evidence out of a literal — the attribute name in getAttribute('data-x'), the '/settings' path, the import specifier — so blanking string contents would delete the evidence. Instead the pattern runs on the string-preserving mask and source_scope.starts_in_code() asks the ANCHOR whether the match START is code. gate-11's route-object brace walk moves to the anchor too, so a '}' inside a literal stops truncating the object the anti-widening guard reads. gate-13 the runner's inline PYMI masked comments with a FOURTH private <!--.*?--> and never masked literals, so 'use <NcDialog> …' in <script> reported an inline modal. Now scopes to vue_markup_mask — the SFC's rendered template — which is the question the rule actually asks. Not a weakening: an inline modal is by definition written in the template. gate-18 tokens were matched against json.dumps(rule), so a rule whose own description WARNED AGAINST the legacy dialect was reported as it, three times. Now walks the structure: three KEY tokens against keys, '@self.' against machine values, with per-locale/documentation fields excluded. Gate had NO helper suite; test_check_notification_dialect.py adds 14 arms. gate-50 #420 hand-rolled _strip_php_comments inside run-hydra-gates.sh, a fourth PHP comment dialect. Deleted; the block calls php_mask. Verified behaviour-preserving over 2481 fleet PHP files (one difference, and it is php_mask being right: a CSS /* inside a string is not a comment). gate-53 stripJsComments was two regexes with no string awareness, so the /* in glob: '/*.vue' opened a block comment that deleted later registry entries — and, when the swallowed span was brace-unbalanced, made parsed:false skip the whole cross-reference check silently. New scripts/lib/js_scope.js is the node port of source_scope.js_comment_mask, asserted BYTE-IDENTICAL to it over a corpus plus this package's own .js sources.
…move its heredoc handling into php_mask #429 (#437) reworked gate-50's window inside the runner while this branch was converting the same block off its private _strip_php_comments. Resolved so BOTH survive: * #429's _method_spans stays, and the runner still clips the guard window at the enclosing method's brace. * _strip_php_comments is gone. The structural copy it fed is now php_mask(src, blank_strings=True); the guard-search copy is php_mask(src). #429 had added HEREDOC handling to the private copy, which php_mask lacked. Deleting the copy without moving that first would have been a capability loss dressed as a cleanup, so php_mask learned heredocs and nowdocs — a strict correctness gain for gates 5, 8, 50, 59 and 64, which until now parsed a <<<SQL body as code (a // inside it blanked a line, an apostrophe opened a literal that ran to the next stray quote, a { mis-balanced every brace walk). Nine arms, four of them mutation-checked EVIDENCE. Also fixes three QUADRATIC scans this branch introduced or inherited, found by timing at three input sizes rather than one: _skip_tag '<a b="' x N -> 12.6 / 134 / 1190 ms (x94) html_comment_spans '{{ ' x N has_prelude/loadApp 'registerAutoloading( ' x N -> 41 / 388 / 3038 ms A tag cannot contain an unquoted '<', so the scan now aborts there; one missed '}}' settles the question for the whole remainder; and the paren walk is a single-pass table instead of a rescan per call site. All paths now measure x5-x17 for a 9x input.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #424.
Order of operations — harden FIRST, consolidate SECOND
The issue is explicit that the obvious consolidation is currently the wrong
move, and the agent who found it said why: "Routing the six private
<!--.*?-->copies intomarkup_masktoday would fix their comment half andleave the delimiter half armed."
So, in two separate commits, in this order:
699b14d— hardensource_scope's comment scope, prove gates 35 and 36flip on the issue's fixture, then redirect the seven private copies onto
the hardened one.
73f1110— close the remaining string-literal axis (gates 10, 11, 13,18, 50, 53).
Within commit 1 the hardening lands above the redirection in the file, so
there is no state in which a checker calls
mask_html_commentswhilemask_html_commentsstill has the hole.1. The delimiter hole, reproduced before it was fixed
Measured on
origin/main@c26f9a3, the issue's fixture verbatim:Gates 35 and 36 over a
.vuecarrying that shape around a live<img alt="">and a livetabindex="3":_HTML_COMMENTis replaced byhtml_comment_spans(), a scanner that knows aquoted attribute value, a Vue
{{ … }}interpolation and a<script>/<style>raw-text body are not comment scope.Deliberately not relaxed, and stated in the code so the next reader does
not mistake them for holes:
<!--in a text node still opens a comment even between quotes — quotescarry no meaning in text and
<p>'<!--'</p>really is a comment to a browser.{{, quote or tag falls back to "this was ordinary text"rather than swallowing the file. Over-blanking is the failure mode this
module exists to remove;
source_scope's header already records one instance(the
<template>-nesting bug that dropped a real<NcSelect>)._HTML_COMMENTstays in the module as the mutation control and nothingmasks with it.
2. Every arm labelled EVIDENCE or CONTROL, by measurement
I ran each suite a second time against a mutated library. The labels below are
what came back, not what I intended:
test_source_scope.py::TestHtmlCommentDelimiterScopehtml_comment_spans = _HTML_COMMENT.finditertest_check_unsafe_auth_resolver.pyphp_mask(src)withoutblank_stringstest_check_unclosable_gate.pytest_check_apphost_autoload_prelude.pyhas_prelude/has_load_appwith no anchortest_check_initial_state.pystarts_in_code -> Truetest_check_admin_router.pystarts_in_code -> Truetest_check_manifest_crossref.jsjs_scoperestored to the two-line regextest_check_notification_dialect.pyjson.dumpsblob scan, kept in-fileThree arms I had labelled CONTROL and had to relabel after measuring — the
measurement is the point of doing it:
test_a_read_quoted_inside_a_string_is_not_a_read_eitherwent red.The pre-fix gate read the sentence as a real read, found no write, and
reported an unclosable gate the app has no code to close.
count assertion is evidence, only the survival assertion is a
control. Split into two arms so each label matches its measurement.
'GlobPage'resolves — a control, not evidence: that entry sitsbefore the phantom opener and resolved either way.
3. Fleet before/after — and why a table of zeroes is not the answer
Six repos, shallow at
development: procest, opencatalogi, openregister,softwarecatalog, docudesk, larpingapp.
Package half:
73d496avs this branch.Real-world delta: zero on every gate. Two rows are non-zero and unchanged —
gate-11 (5/1/1/3/1/2) and gate-7 (27/10/8/1/17/1, which I do not touch).
harness is reading real trees rather than empty file lists.
markup_maskover procest's 231 real.vuefiles yields 5,294 open tagsand is byte-identical before and after.
demotion notes, docudesk 1, across 753
(Controller|Service)*.phpfiles, zero stderr on both arms — and unchanged against a base that now
includes fix(gate-50): the guard window crossed method boundaries (#429) #437's method-span clip.
heredoc, 8 openers in total. A
0 -> 0over files containing none would havebeen a check with no subject, not a check that passed.
Two-directional control on a reconstructed pre-fix tree. Inject the issue's
delimiter pair into the template of every real component, then count the
elements each mask still lets a gate see:
7,321 real elements would have been invisible to gates 31/32/35/36/37/39/
40/42/43/44 on live fleet markup. The post-fix column loses none: every
negative number is exactly
2 × files— the two injected<p>elementsthemselves (231×2=462, 93×2=186, 205×2=410, 73×2=146, 72×2=144, 5×2=10). That
arithmetic identity is itself a check that the harness measured what it claims.
Where the heredoc port bites.
php_maskcompared over all 2,481 fleet PHPfiles, old vs new: 2 files differ in the default mask, 5 in the
structural one. Both default-mode differences are the same real bug being
fixed — CSS inside a heredoc, where
#was read as a PHP comment and blankedthe rest of the line:
Note the
{in the first: previously swallowed by a comment that never opened;now visible in the default mask and correctly blanked in the structural one, so
the brace walk is right either way.
3b. Three QUADRATIC scans, found only because I timed at three sizes
A sibling PR in this batch shipped a HIGH
py/redos, so I timed every patternI added or grew. A single measurement would have shown nothing — all three
look instant at one size:
_skip_tagon'<a b="' * Nhtml_comment_spanson'{{ ' * Nhas_preludeon'registerAutoloading( ' * NA 9× input should cost about 9×. The first extrapolates to roughly three
hours on a 1 MB file, and a gate that times out is a gate that did not run —
this programme's own failure mode arriving by a new route.
None is exponential backtracking, so
py/redoswould not have flagged any ofthem; two are my own new code and one (
[^)]*) I copied from a shape alreadyon
main. All three fixes are "stop rescanning": a tag cannot contain anunquoted
<so_skip_tagaborts there; one missed}}settles the questionfor the whole remainder; and the parenthesis walk is a single-pass table
instead of a fresh walk per call site.
CodeQL: 0 open alerts on this branch. Positive control on the query — the
same call returns 2 alerts repo-wide on
main, so the empty list is a realclean rather than a broken query.
3c. Default paging really does hide checks
Verified on this PR:
/commits/<sha>/check-runsreportstotal_count=34andreturns 30 without
?per_page=100. The four in the dropped tail:Three of the four are the CodeQL analyses — the exact check that caught the
sibling ReDoS. Read properly: 16 success, 17 skipped, 0 failed.
4. What each gate now does
php_mask(blank_strings=True). Nothing it matches is ever a string, and blanking also repairs the brace walker, which previously counted a{written inside a literal as a block delimiter.source_scope.starts_in_code(). The mask cannot blank contents — the name ingetAttribute('data-x')and thedata-requesttokenexemption are read out of that literal — so the ANCHOR answers "is the expression code?" instead.}inside a literal no longer truncates the object the anti-widening guard is read out of.PYMIcarried a fourth private<!--.*?-->and no string awareness. Now scopes tovue_markup_mask— the SFC's rendered template — which is the question the rule actually asks. Not a weakening: an inline modal is by definition written in the template.json.dumps(rule), so a rule whose owndescriptionwarned against the legacy dialect was reported as it, three times. Now walks the structure: three KEY tokens against keys,@self.against machine values, per-locale/documentation fields excluded.<!--.*?-->copies →mask_html_comments_strip_php_commentsdeleted; the block callsphp_mask.stripJsCommentswas two regexes with no string awareness, so the/*inglob: '/*.vue'opened a block comment that deleted later registry entries — and when the swallowed span was brace-unbalanced,parsed: falsemade the caller skip the cross-reference check entirely, a false GREEN. Newscripts/lib/js_scope.jsis the node port ofsource_scope.js_comment_mask.strip_comments()— #184's own state machine — now delegates tophp_mask, removing a second PHP comment dialect.Also consolidated, same class, one line:
check_orphaned_write_capability's<!--.*?-->overinfo.xml.On the new
js_scope.jsThe node checkers had no shared scope module, so gate-53's fix would have been
a third private copy. Instead it is a port with a drift test:
test_js_scope.jsshells out tosource_scope.py --mask js-commentsandasserts the two are byte-identical over a corpus and over this package's
own
.jssources. That is the tradetest_source_scope.py::TestSharedWithGate19already makes — two copies provenequal are a maintenance cost, two copies that might differ are a defect.
check_notification_dialecthad no helper suite at all; this adds one with14 arms, including the pre-#424 blob scan kept in-file as the mutation control.
5. Residuals — stated rather than left looking closed
class_exists_targets/openregister_probesare notanchored.
openregister_probesbuilds its text byheader + bodystringconcatenation, which destroys offsets, so threading an anchor through it is
a restructure, not a parameter. The axis String literals count as evidence in 16 gates — and 8 of them are ONE hole in source_scope.markup_mask #424 names for gate-64 (a quoted
prelude closing the gate) is closed; a quoted
class_exists('OCA\OpenRegister\AppHost\X')inside prose can still producea non-blocking NOTE.
BOOTSTRAP_REFstill matches inside strings, on purpose. Thequoted FQCN form is documented evidence, so anchoring it would lose real
findings. That is 12 gates where a comment MANUFACTURES a finding (#415 class, false-positive half) #423's axis, not this one.
check_no_admin_idor._strip_strings_and_comments(gate-7) is NOTconsolidated, and this is a trade, not an omission. It handles
heredocs, which
php_maskdoes not. Routing gate-7 ontophp_masktoday would drop heredoc handling from the fleet's highest-yield security
gate to remove a duplicate — the wrong side of that trade. The right fix is
to teach
php_maskheredocs and then consolidate, which widens the blastradius to gates 5, 8, 50, 59 and 64. fix(gates 7, 25, 48, 49): a comment must not be enough to get past a gate — 47 of 65 were (#415 class) #425 is also live on gate-7.
kind:lookup uses a fixed 400-character window. Comments arenow blanked in place rather than collapsed to one space, so a long comment
between a registry key and its
kind:could pushkindout of that window.No instance in the fleet fixtures; the window was already a heuristic in
both directions.
6. Suites — including a false baseline I caught and corrected
🔴 My first baseline was contaminated by my own edits and I had written it up
as a pre-existing failure. I started
run-helper-suites.shin the backgroundon a fresh
mainclone and then began editingsource_scope.pyin that sameworking tree. The run takes ~40 minutes, so
test_gate_license_triangle_scope.shexecuted after my half-finished editslanded and reported:
Re-run on a pristine, untouched
mainclone:passed: 11, failed: 0. Notpre-existing. Mine.
The tell was in the text and I nearly walked past it: the passing and failing
messages carry the same status number (98) and differ only in the verb —
"DID fail" vs "did NOT fail". A matching number is not agreement.
A background suite run and an edit session cannot share a working tree. The
run reads each file at the moment it reaches it, not at the moment it starts,
so "I started it before I edited" is no defence.
So the numbers below are CI's, not my machine's — the
hydra-gates package / Package invariantsjob runs the samerun-helper-suites.shin a pristine checkout nothing else is writing to, whichis exactly the property my local run lacked:
main@c26f9a3main@5543c2dmain@73d496aebfa5d3Every digit accounted for.
73d496aadds no new suite file — it onlymodifies
test_gate_45_to_55_acceptance.sh— so its tally is5543c2d's 84.This PR adds exactly two files (
test_check_notification_dialect.py,test_js_scope.js), which the runner auto-discovers.84 + 2 = 86, which iswhat CI measured. Nothing disappeared in either direction.
main@73d496a's own run was CANCELLED, so it is not usable as thebaseline — a cancelled job is no verdict. The comparison therefore goes
through
5543c2dplus a file-level diff rather than through a run that neverfinished.
test_gate_45_to_55_acceptance.sh(which covers gate-50) is ALL GREEN onthe merged head, including #437's own new arms.
Working notes and every number:
/home/rubenlinde/fleet-board/findings/gates-424-library.md🤖 Generated with Claude Code
7. The
#437merge — what I kept and what I moved#437reworked gate-50's window inside the runner while this branch wasconverting the same block off its private
_strip_php_comments. Both changesare right, so both survive:
#437's_method_spansstays. The guard window is still clipped at theenclosing method's brace, and its acceptance arms pass on the merged head —
including "a
}in a string or heredoc does not truncate the method span",which is the arm that proves the capability moved rather than vanished.
_strip_php_commentsis gone. The structural copy it fed is nowphp_mask(src, blank_strings=True); the guard-search copy isphp_mask(src).php_maskhad to learn heredocs first.#437had added heredoc handlingto the private copy, and the library did not have it. Deleting the copy without
moving that capability would have been a capability loss dressed as a
cleanup — precisely the shape this programme exists to stop. So
php_masknow understands heredocs and nowdocs, which is a strict correctness gain for
all five of its gates (5, 8, 50, 59, 64): until now a
<<<SQLbody was parsedas code, so a
//or#inside it blanked a line, an apostrophe opened aliteral that ran to the next stray quote, and a
{mis-balanced every bracewalk built on the mask. Nine new arms, four of them mutation-checked EVIDENCE.
A pleasant side effect: the reason I had recorded for not consolidating
gate-7 was that its private stripper handles heredocs and
php_maskdid not.That objection is now gone, so gate-7 is a clean follow-up rather than a trade.
test_gate_45_to_55_acceptance.sh— which covers gate-50 — is ALL GREEN onthe merged head.