Skip to content

Commit 3e595a0

Browse files
Anchor the fence closer grammar; make the fence tests pin the scanner
Two r4 findings: P1 (leading tab accepted before a closing fence): the closer computed indentation with lstrip(' ') (spaces only) but the delimiter with strip() (tabs too), so a TAB + triple-backtick line closed the block and exposed the fenced count as top-level metadata — reproduced as a false clean POST. The closer is now an anchored grammar: 0-3 LITERAL leading spaces (a leading tab is 4 columns, i.e. content), the matching delimiter repeated at least the opening length, and only [ \t]* afterward. Entry-point cases for tab and mixed space/tab indentation assert exit 1 and zero POSTs. P2 (test-oracle weakness): the six r3 regression tests stayed green with the broken scanner restored — their fixtures placed the malicious fence AFTER the Review Summary section, so the positional guard rejected the exposed row regardless of scanner correctness. The fixtures now place the fence in the metadata slot (or the fake heading ahead of the real summary for the tilde variant), so a naive toggling scanner WOULD promote the fenced row into the official position. Mutation-verified locally: the r3 naive-toggling scanner fails all 10 fence/tab tests, and the r4 tab-closer bug fails exactly the 4 tab tests; the fixed scanner passes all 87. Verification: python3 -m unittest discover -s .github/actions/pr-review/scripts -p 'test_*.py' — 87 tests pass; both mutants above fail as named. Co-authored-by: c1-squire-dev[bot] <c1-squire-dev[bot]@users.noreply.github.com>
1 parent 21b2a1c commit 3e595a0

2 files changed

Lines changed: 87 additions & 31 deletions

File tree

‎.github/actions/pr-review/scripts/submit-verdict-review.py‎

Lines changed: 14 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -154,6 +154,10 @@ def sha_bound_to_head(reviewed: str | None, head: str) -> bool:
154154
# A fence opener: up to 3 leading spaces, then 3+ backticks or tildes, then an
155155
# optional info string (CommonMark 0.31.2, fenced code blocks).
156156
_FENCE_OPEN_PATTERN = re.compile(r"^ {0,3}(`{3,}|~{3,})(.*)$")
157+
# A fence closer: up to 3 LITERAL leading spaces (a leading tab is 4 columns —
158+
# content, not a closer), then a delimiter run, then only spaces/tabs. The
159+
# delimiter character and minimum length are checked against the opener.
160+
_FENCE_CLOSE_PATTERN = re.compile(r"^ {0,3}(`+|~+)[ \t]*$")
157161

158162

159163
def _top_level_lines(body: str) -> list[str]:
@@ -184,17 +188,16 @@ def _top_level_lines(body: str) -> list[str]:
184188
continue
185189
lines.append(line)
186190
continue
187-
# Inside a fence: only a valid closer ends it.
188-
indent = len(line) - len(line.lstrip(" "))
189-
stripped = line.strip()
190-
if (
191-
indent <= 3
192-
and stripped
193-
and set(stripped) == {fence_char}
194-
and len(stripped) >= fence_len
195-
):
196-
fence_char = None
197-
fence_len = 0
191+
# Inside a fence: only a valid closer ends it. The closer grammar is
192+
# anchored: 0-3 literal leading spaces (a leading tab is 4 columns,
193+
# i.e. content), the matching delimiter repeated at least the opening
194+
# length, and only spaces/tabs afterward.
195+
closer = _FENCE_CLOSE_PATTERN.match(line)
196+
if closer:
197+
delimiter = closer.group(1)
198+
if delimiter[0] == fence_char and len(delimiter) >= fence_len:
199+
fence_char = None
200+
fence_len = 0
198201
# Fence openers/closers and fenced content are never top-level lines.
199202
return lines
200203

‎.github/actions/pr-review/scripts/test_verdict_scaffolding.py‎

Lines changed: 73 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -184,23 +184,45 @@ def test_out_of_position_row_rejected(self):
184184

185185
def test_longer_fence_embedded_shorter_run_is_content(self):
186186
# A triple-backtick line inside a four-backtick fence is content, not
187-
# a closer; the row after it stays fenced.
188-
body = summary_body(0).replace(count_row(0) + "\n", "")
189-
body += "\n````markdown\n```\n" + count_row(0) + "\n```\n````\n"
187+
# a closer. The fence sits in the metadata slot, so a naive toggling
188+
# scanner WOULD promote the fenced row into the official position —
189+
# this fixture fails on that broken scanner, not just on fixed code.
190+
body = summary_body(0).replace(
191+
count_row(0), "````markdown\n```\n" + count_row(0) + "\n```\n````"
192+
)
190193
self.assertIsNone(sv.parse_blocking_count(body, HEADING))
191194

192195
def test_closer_with_info_suffix_is_not_a_closer(self):
193196
# "```example" inside a fence is content (a closer may only have
194-
# trailing whitespace); the row after it stays fenced.
195-
body = summary_body(0).replace(count_row(0) + "\n", "")
196-
body += "\n```\n ```example\n" + count_row(0) + "\n```\n"
197+
# trailing whitespace). Metadata-slot placement: a naive scanner
198+
# treats it as a closer and accepts the exposed row.
199+
body = summary_body(0).replace(
200+
count_row(0), "```\n ```example\n" + count_row(0) + "\n```"
201+
)
197202
self.assertIsNone(sv.parse_blocking_count(body, HEADING))
198203

199204
def test_tilde_fence_hides_fake_heading_and_row(self):
200205
# Tilde fences are fences too: a fake heading + count inside one can
201-
# never supply the verdict.
202-
body = summary_body(0).replace(count_row(0) + "\n", "")
203-
body += "\n~~~markdown\n### Connector PR Review: fake\n\n" + count_row(0) + "\n~~~\n"
206+
# never supply the verdict. The fake heading precedes the real
207+
# summary, so a backtick-only scanner finds the fake pair and accepts.
208+
fake = "~~~markdown\n### Connector PR Review: fake\n\n" + count_row(0) + "\n~~~\n"
209+
body = fake + summary_body(0).replace(count_row(0) + "\n", "")
210+
self.assertIsNone(sv.parse_blocking_count(body, HEADING))
211+
212+
def test_tab_indented_closer_is_content(self):
213+
# A leading tab is 4 columns — the line is content, not a closer, so
214+
# the row after it stays fenced. A scanner that strips the tab into a
215+
# valid delimiter accepts the exposed row here.
216+
body = summary_body(0).replace(
217+
count_row(0), "```\n\t```\n" + count_row(0) + "\n```"
218+
)
219+
self.assertIsNone(sv.parse_blocking_count(body, HEADING))
220+
221+
def test_space_tab_indented_closer_is_content(self):
222+
# Space-then-tab before a closing fence is likewise content.
223+
body = summary_body(0).replace(
224+
count_row(0), "```\n \t```\n" + count_row(0) + "\n```"
225+
)
204226
self.assertIsNone(sv.parse_blocking_count(body, HEADING))
205227

206228

@@ -447,11 +469,13 @@ def test_malformed_real_row_plus_fenced_row_fails_closed(self):
447469
self.assertEqual(posted, [])
448470

449471
def test_four_backtick_embedded_triple_fails_closed(self):
450-
# r3 variant (a): a four-backtick block containing a triple-backtick
451-
# line and a canonical zero row, with no real metadata row. The
452-
# embedded shorter run is content, not a closer.
453-
body = summary_body(0).replace(count_row(0) + "\n", "")
454-
body += "\n````markdown\n```\n" + count_row(0) + "\n```\n````\n"
472+
# r3 variant (a): a four-backtick block in the metadata slot
473+
# containing a triple-backtick line and a canonical zero row. The
474+
# embedded shorter run is content, not a closer; a naive toggling
475+
# scanner promotes the fenced row into the official slot and POSTs.
476+
body = summary_body(0).replace(
477+
count_row(0), "````markdown\n```\n" + count_row(0) + "\n```\n````"
478+
)
455479
posted = []
456480
code, _ = self._run_main(
457481
[comment(7, body)], rest_side_effect=self._rest_dispatch(posted=posted)
@@ -461,9 +485,11 @@ def test_four_backtick_embedded_triple_fails_closed(self):
461485

462486
def test_invalid_closer_suffix_fails_closed(self):
463487
# r3 variant (b): a line beginning "```example" inside a fenced block
464-
# is not a valid closer; the row after it stays fenced.
465-
body = summary_body(0).replace(count_row(0) + "\n", "")
466-
body += "\n```\n ```example\n" + count_row(0) + "\n```\n"
488+
# is not a valid closer; the row after it stays fenced. Metadata-slot
489+
# placement pins the broken scanner.
490+
body = summary_body(0).replace(
491+
count_row(0), "```\n ```example\n" + count_row(0) + "\n```"
492+
)
467493
posted = []
468494
code, _ = self._run_main(
469495
[comment(7, body)], rest_side_effect=self._rest_dispatch(posted=posted)
@@ -473,9 +499,36 @@ def test_invalid_closer_suffix_fails_closed(self):
473499

474500
def test_tilde_fenced_fake_summary_fails_closed(self):
475501
# r3 variant (c): a fake heading + canonical row inside a tilde fence
476-
# can never supply the verdict.
477-
body = summary_body(0).replace(count_row(0) + "\n", "")
478-
body += "\n~~~markdown\n### Connector PR Review: fake\n\n" + count_row(0) + "\n~~~\n"
502+
# can never supply the verdict. The fake pair precedes the real
503+
# summary so a backtick-only scanner accepts it.
504+
fake = "~~~markdown\n### Connector PR Review: fake\n\n" + count_row(0) + "\n~~~\n"
505+
body = fake + summary_body(0).replace(count_row(0) + "\n", "")
506+
posted = []
507+
code, _ = self._run_main(
508+
[comment(7, body)], rest_side_effect=self._rest_dispatch(posted=posted)
509+
)
510+
self.assertEqual(code, 1)
511+
self.assertEqual(posted, [])
512+
513+
def test_tab_indented_closer_fails_closed(self):
514+
# r4 variant: a TAB before the closing fence makes the line content
515+
# (4 columns), not a closer; the exposed row must not be submitted.
516+
body = summary_body(0).replace(
517+
count_row(0), "```\n\t```\n" + count_row(0) + "\n```"
518+
)
519+
posted = []
520+
code, _ = self._run_main(
521+
[comment(7, body)], rest_side_effect=self._rest_dispatch(posted=posted)
522+
)
523+
self.assertEqual(code, 1)
524+
self.assertEqual(posted, [])
525+
526+
def test_space_tab_indented_closer_fails_closed(self):
527+
# r4 variant: space-then-tab before the closing fence is likewise
528+
# content, not a closer.
529+
body = summary_body(0).replace(
530+
count_row(0), "```\n \t```\n" + count_row(0) + "\n```"
531+
)
479532
posted = []
480533
code, _ = self._run_main(
481534
[comment(7, body)], rest_side_effect=self._rest_dispatch(posted=posted)

0 commit comments

Comments
 (0)