Skip to content

Commit 58befcf

Browse files
author
CometAPI
committed
fix: close release evidence parser bypasses
1 parent 862eca9 commit 58befcf

2 files changed

Lines changed: 134 additions & 87 deletions

File tree

scripts/_checks.py

Lines changed: 70 additions & 84 deletions
Original file line numberDiff line numberDiff line change
@@ -14,12 +14,11 @@
1414
from datetime import date
1515
from email.message import Message
1616
from email.parser import Parser
17+
from html.parser import HTMLParser
1718
from pathlib import Path
1819
from typing import cast
1920
from urllib.parse import quote, unquote
2021

21-
from markdown_it import MarkdownIt
22-
2322
if sys.version_info >= (3, 11):
2423
import tomllib
2524
else:
@@ -134,80 +133,22 @@
134133
r")(?=$|[/?#>])"
135134
)
136135
_HTTP_URL = re.compile(r"https?://[^\s<>)\]]+", re.IGNORECASE)
137-
_URLISH_TOKEN = re.compile(r"https?:[^\s<>\"']+", re.IGNORECASE)
138136
_RECOVERY_TAGS = {"0.1.0a1": "v0.1.0-alpha.1+recovery.1"}
139137
_FULL_COMMIT = re.compile(r"(?<![0-9a-f])[0-9a-f]{40}(?![0-9a-f])", re.IGNORECASE)
140138
_ACTIONS_PREFIX = f"{CANONICAL_REPOSITORY}/actions/runs/"
141139
_ACTIONS_PATH = re.compile(
142140
r"actions/runs/(?P<run>\d+)(?:/attempts/(?P<attempt>\d+))?",
143141
re.IGNORECASE,
144142
)
145-
_CANONICAL_ACTIONS_URL = re.compile(
146-
rf"(?<![^\s<(\"']){re.escape(_ACTIONS_PREFIX)}(?P<run>[1-9]\d*)"
143+
_CANONICAL_ACTIONS_DESTINATION = re.compile(
144+
rf"{re.escape(_ACTIONS_PREFIX)}(?P<run>[1-9]\d*)"
147145
r"(?:/attempts/(?P<attempt>[1-9]\d*))?"
148-
r"(?=$|[\s<>\"')\]}]|[.,;:!?](?=$|\s))"
149146
)
150-
_RAW_HTML_URL_ATTRIBUTE = re.compile(
151-
r"(?is)\b(?:href|src)\s*=\s*(?P<quote>['\"])(?P<url>.*?)(?P=quote)"
147+
_RAW_ACTIONS_DESTINATION = re.compile(
148+
rf"(?<![^\s(])(?P<url>{re.escape(_ACTIONS_PREFIX)}[1-9]\d*"
149+
r"(?:/attempts/[1-9]\d*)?)"
150+
r"(?=$|[\s.,;:!?])"
152151
)
153-
154-
155-
def _actions_path_has_canonical_url(
156-
text: str,
157-
path: re.Match[str],
158-
canonical_matches: list[re.Match[str]],
159-
) -> bool:
160-
canonical = next(
161-
(
162-
match
163-
for match in canonical_matches
164-
if match.start() <= path.start() and path.end() <= match.end()
165-
),
166-
None,
167-
)
168-
if canonical is None:
169-
return False
170-
171-
for link in _MARKDOWN_LINK.finditer(text):
172-
if link.start("target") <= path.start() and path.end() <= link.end("target"):
173-
return _CANONICAL_ACTIONS_URL.fullmatch(link.group("target")) is not None
174-
175-
for attribute in _RAW_HTML_URL_ATTRIBUTE.finditer(text):
176-
if attribute.start("url") <= path.start() and path.end() <= attribute.end("url"):
177-
return _CANONICAL_ACTIONS_URL.fullmatch(attribute.group("url")) is not None
178-
179-
containing_tokens = [
180-
match
181-
for match in _URLISH_TOKEN.finditer(text)
182-
if match.start() <= path.start() and path.end() <= match.end()
183-
]
184-
if containing_tokens and min(match.start() for match in containing_tokens) < canonical.start():
185-
return False
186-
187-
if canonical.start() == 0:
188-
return True
189-
boundary = text[canonical.start() - 1]
190-
if boundary.isspace():
191-
return True
192-
if boundary == "<":
193-
return canonical.start() >= 2 and text[canonical.start() - 2].isspace()
194-
if boundary == "(":
195-
return canonical.start() == 1 or text[canonical.start() - 2].isspace()
196-
return False
197-
198-
199-
def _rendered_markdown_link_targets(text: str) -> list[str]:
200-
targets: list[str] = []
201-
for token in MarkdownIt("commonmark", {"html": True}).parse(text):
202-
for child in token.children or []:
203-
if child.type != "link_open":
204-
continue
205-
target = child.attrGet("href")
206-
if isinstance(target, str):
207-
targets.append(target)
208-
return targets
209-
210-
211152
_WHEEL_DIGEST = re.compile(
212153
r"\bwheel\s+sha256\b[^0-9a-f]{0,96}(?P<digest>[0-9a-f]{64})(?![0-9a-f])",
213154
re.IGNORECASE | re.DOTALL,
@@ -250,6 +191,24 @@ class CheckError(RuntimeError):
250191
"""Raised when release-candidate evidence does not satisfy a local gate."""
251192

252193

194+
class _ActionsAnchorParser(HTMLParser):
195+
"""Collect exact Actions destinations from rendered HTML anchors."""
196+
197+
def __init__(self) -> None:
198+
super().__init__(convert_charrefs=True)
199+
self.destinations: Counter[tuple[str, str | None]] = Counter()
200+
201+
def handle_starttag(self, tag: str, attrs: list[tuple[str, str | None]]) -> None:
202+
if tag.casefold() != "a":
203+
return
204+
for name, value in attrs:
205+
if name.casefold() != "href" or value is None:
206+
continue
207+
destination = _CANONICAL_ACTIONS_DESTINATION.fullmatch(value)
208+
if destination is not None:
209+
self.destinations[(destination.group("run"), destination.group("attempt"))] += 1
210+
211+
253212
@dataclass(frozen=True)
254213
class ReleaseEvidenceIdentity:
255214
"""Machine-readable identity for one immutable historical release."""
@@ -747,53 +706,80 @@ def _canonical_actions_run_violations(
747706
findings: list[tuple[int, str]] = []
748707
direct = unicodedata.normalize("NFKC", body)
749708
variants = [direct]
750-
for _ in range(8):
709+
converged = False
710+
for _ in range(len(direct) + 1):
751711
previous = variants[-1]
752712
decoded = html.unescape(previous)
753713
decoded = unquote(decoded)
754714
decoded = re.sub(r"\\([/\\.:?&=%#])", r"\1", decoded)
755-
decoded = decoded.replace("\t", "").replace("\n", "").replace("\r", "")
715+
decoded = decoded.replace("\t", "")
756716
decoded = re.sub(
757717
r"(?i)(https?:[^\s<>)\]]*)\\([^\s<>)\]]*)",
758718
lambda match: match.group(0).replace("\\", "/"),
759719
decoded,
760720
)
761721
if decoded == previous:
722+
converged = True
762723
break
763724
variants.append(decoded)
764725

765-
path_matches = list(_ACTIONS_PATH.finditer(direct))
766-
canonical_matches = list(_CANONICAL_ACTIONS_URL.finditer(direct))
767-
malformed = any(
768-
not _actions_path_has_canonical_url(direct, path, canonical_matches)
769-
for path in path_matches
770-
)
771-
direct_paths = Counter((match.group("run"), match.group("attempt")) for match in path_matches)
772726
variant_paths = [
773727
Counter(
774-
(match.group("run"), match.group("attempt"))
728+
(match.group("run"), match.group("attempt") or None)
775729
for match in _ACTIONS_PATH.finditer(variant)
776730
)
777731
for variant in variants
778732
]
733+
normalized = variants[-1]
734+
rendered_destinations: Counter[tuple[str, str | None]] = Counter()
735+
# Keep module import dependency-free for the copied-checkout pre-sync bootstrap.
736+
from markdown_it import MarkdownIt
737+
738+
parser = MarkdownIt("commonmark", {"html": True})
739+
for token in parser.parse(normalized):
740+
for child in token.children or []:
741+
if child.type == "html_inline":
742+
html_parser = _ActionsAnchorParser()
743+
html_parser.feed(child.content)
744+
rendered_destinations.update(html_parser.destinations)
745+
continue
746+
if child.type != "link_open":
747+
continue
748+
target = child.attrGet("href")
749+
if not isinstance(target, str):
750+
continue
751+
destination = _CANONICAL_ACTIONS_DESTINATION.fullmatch(target)
752+
if destination is not None:
753+
rendered_destinations[(destination.group("run"), destination.group("attempt"))] += 1
754+
755+
if token.type == "html_block":
756+
html_parser = _ActionsAnchorParser()
757+
html_parser.feed(token.content)
758+
rendered_destinations.update(html_parser.destinations)
759+
760+
for raw in _RAW_ACTIONS_DESTINATION.finditer(normalized):
761+
destination = _CANONICAL_ACTIONS_DESTINATION.fullmatch(raw.group("url"))
762+
if destination is not None:
763+
rendered_destinations[(destination.group("run"), destination.group("attempt"))] += 1
764+
765+
malformed = not converged or variant_paths[-1] != rendered_destinations
779766
malformed = malformed or any(
780-
_ACTIONS_PATH.search(target) is not None
781-
and _CANONICAL_ACTIONS_URL.fullmatch(target) is None
782-
for variant in variants
783-
for target in _rendered_markdown_link_targets(variant)
784-
)
785-
malformed = malformed or any(
786-
count > direct_paths[identity]
767+
count > variant_paths[0][identity]
787768
for paths in variant_paths[1:]
788769
for identity, count in paths.items()
789770
)
771+
malformed = malformed or any(
772+
match.end() < len(normalized) and normalized[match.end()] not in " \t\r\n.,;:!?)]}>\"'"
773+
for match in _ACTIONS_PATH.finditer(normalized)
774+
)
790775

791776
if malformed:
792777
findings.append(
793778
(
794779
line,
795780
f"release-evidence block for {version} contains a non-canonical Actions URL; "
796-
"use the exact repository /actions/runs/<positive-id> URL with an optional "
781+
"use the exact repository /actions/runs/<positive-id> URL as plain Markdown "
782+
"text, an autolink, or a link destination, with an optional "
797783
"/attempts/<positive-id> suffix",
798784
)
799785
)

tests/test_release_documents.py

Lines changed: 64 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@
77
import zipfile
88
from collections.abc import Callable
99
from pathlib import Path
10+
from urllib.parse import quote
1011

1112
import pytest
1213

@@ -705,11 +706,18 @@ def test_release_evidence_rejects_obsolete_workflow_reference_marker(
705706
"https://evil.example/#(https://github.com/cometapi-dev/cometapi-python/"
706707
"actions/runs/30515861246)",
707708
"mailto:(https://github.com/cometapi-dev/cometapi-python/actions/runs/30515861246)",
708-
"prefix<https://github.com/cometapi-dev/cometapi-python/actions/runs/30515861246>",
709709
"[evil](mailto:foo](https://github.com/cometapi-dev/cometapi-python/"
710710
"actions/runs/30515861246))",
711711
"[evil](mailto:foo([x](https://github.com/cometapi-dev/cometapi-python/"
712712
"actions/runs/30515861246)))",
713+
"mailto:x](https://github.com/cometapi-dev/cometapi-python/actions/runs/30515861246)",
714+
"[release](mailto:evil (https://github.com/cometapi-dev/cometapi-python/"
715+
"actions/runs/30515861246))",
716+
"![release](https://github.com/cometapi-dev/cometapi-python/actions/runs/30515861246)",
717+
'<a href="mailto:evil" title="see (https://github.com/cometapi-dev/'
718+
'cometapi-python/actions/runs/30515861246)">release run</a>',
719+
'<form action="https://github.com/cometapi-dev/cometapi-python/actions/runs/'
720+
'30515861246">release run</form>',
713721
"http://github.com/cometapi-dev/cometapi-python/actions/runs/30511373822",
714722
"https://evil.example/?next=https%3A%2F%2Fgithub.com%2Fcometapi-dev%2F"
715723
"cometapi-python%2Factions%2Fruns%2F30511373822",
@@ -754,8 +762,40 @@ def test_release_evidence_rejects_noncanonical_workflow_reference_url(
754762
with pytest.raises(CheckError) as caught:
755763
require_public_preview_docs()
756764

757-
assert "non-canonical Actions URL" in str(caught.value)
758-
assert "/actions/runs/<positive-id>" in str(caught.value)
765+
message = str(caught.value)
766+
assert (
767+
"non-canonical Actions URL" in message
768+
or "contradicts its release-identity marker" in message
769+
)
770+
if "non-canonical Actions URL" in message:
771+
assert "/actions/runs/<positive-id>" in message
772+
773+
774+
@pytest.mark.parametrize("depth", [9, 64])
775+
def test_release_evidence_rejects_deeply_encoded_workflow_reference_url(
776+
releasable_documents: Path,
777+
depth: int,
778+
) -> None:
779+
wrapped = (
780+
"https://evil.example/?next=https://github.com/cometapi-dev/cometapi-python/"
781+
"actions/runs/30511373822"
782+
)
783+
for _ in range(depth):
784+
wrapped = quote(wrapped, safe="")
785+
evidence = _release_evidence_block().replace(
786+
"30515861246\n- https://pypi.org",
787+
f"30515861246\n- Required CI {wrapped}\n- https://pypi.org",
788+
1,
789+
)
790+
for name in ("ROADMAP.md", "RELEASING.md"):
791+
with (releasable_documents / name).open("a", encoding="utf-8") as stream:
792+
stream.write(evidence)
793+
794+
with pytest.raises(CheckError) as caught:
795+
require_public_preview_docs()
796+
797+
message = str(caught.value)
798+
assert "non-canonical Actions URL" in message
759799

760800

761801
def test_release_evidence_accepts_canonical_raw_html_anchor(
@@ -1512,6 +1552,27 @@ def test_copied_repository_verification_runs_public_document_gate() -> None:
15121552
]
15131553

15141554

1555+
def test_copied_repository_checker_imports_before_dependency_sync(
1556+
tmp_path: Path,
1557+
) -> None:
1558+
root = tmp_path / "repository"
1559+
scripts = root / "scripts"
1560+
scripts.mkdir(parents=True)
1561+
for name in ("_checks.py", "check_repository_independence.py"):
1562+
shutil.copy2(PROJECT_ROOT / "scripts" / name, scripts / name)
1563+
1564+
result = subprocess.run(
1565+
[sys.executable, "-S", "scripts/check_repository_independence.py", "--scan-only"],
1566+
cwd=root,
1567+
capture_output=True,
1568+
check=False,
1569+
text=True,
1570+
)
1571+
1572+
assert result.returncode == 0, result.stderr
1573+
assert "standalone copied-checkout verification passed" in result.stdout
1574+
1575+
15151576
def test_artifact_metadata_must_match_source_readme_exactly() -> None:
15161577
expected = (
15171578
"Stable 0.1.x maintenance releases are available from PyPI.\n"

0 commit comments

Comments
 (0)