From 40c3ece3921849a9d1f42fe9483f536e0bff98ab Mon Sep 17 00:00:00 2001 From: Pieter Viljoen Date: Sat, 22 Aug 2026 09:21:30 -0700 Subject: [PATCH 1/3] Reject a Query String or Fragment in a Registry URL ## Summary - CodeRabbit caught a real gap on the promotion PR (#910): both GITHUB_URL_RE copies excluded only '/' and whitespace from the owner/repo components, so a url like "https://github.com/owner/repo?tab=readme" matched with repo name "repo?tab=readme" instead of failing to match. That identity can never equal GitHub's real full_name, so membership_findings() would report a false DEFECT for a repo the registry actually carries. Excluded '?' and '#' from both character classes in both files, so a query string or fragment now correctly fails to parse (spec/validate.py's own non-canonical-url check then rejects the entry, same as any other malformed url). ## Verification - `python3 spec/audit.py --selftest`: SELFTEST PASS, including new repo_identity() cases for a query string, a fragment, and both combined with a .git suffix, all now rejected - `python3 spec/validate.py`: OK against the real registry - `python3 scripts/prose_lint.py`: 0 issues - `python3 scripts/repo_gate.py --check {eol,eol-coverage,sha-pin}`: 0 issues each - `ruff check` / `ruff format --check`: clean - `mypy spec/audit.py spec/validate.py`: no issues --- spec/audit.py | 8 ++++++-- spec/validate.py | 3 ++- 2 files changed, 8 insertions(+), 3 deletions(-) diff --git a/spec/audit.py b/spec/audit.py index 48e9a3877..ff03f754c 100755 --- a/spec/audit.py +++ b/spec/audit.py @@ -255,7 +255,7 @@ def owner_repos(owner): page += 1 -GITHUB_URL_RE = re.compile(r"^https://github\.com/([^/\s]+)/([^/\s]+?)(?:\.git)?/?$") +GITHUB_URL_RE = re.compile(r"^https://github\.com/([^/\s?#]+)/([^/\s?#]+?)(?:\.git)?/?$") def repo_identity(url): @@ -263,7 +263,8 @@ def repo_identity(url): The canonical identity two repos are compared by, since a bare repo name collides across owners and GitHub's own full_name field is already this exact shape (case-preserved). A - trailing .git is stripped so it still resolves to the same identity full_name would. + trailing .git is stripped so it still resolves to the same identity full_name would, and a + query string or fragment is rejected outright rather than folded into the repo name. spec/validate.py rejects a url that fails to parse here at all, kept in sync with this regex. """ if not isinstance(url, str): @@ -4212,6 +4213,9 @@ def _selftest(): (" https://github.com/owner/Repo ", "owner/repo"), ("https://github.com/owner/Repo.git", "owner/repo"), ("https://github.com/owner/Repo.github", "owner/repo.github"), + ("https://github.com/owner/Repo?tab=readme", None), + ("https://github.com/owner/Repo#readme", None), + ("https://github.com/owner/Repo.git?tab=readme", None), ("http://github.com/owner/Repo", None), ("https://gitlab.com/owner/Repo", None), ("not a url", None), diff --git a/spec/validate.py b/spec/validate.py index e838ed2e4..60f5ae132 100755 --- a/spec/validate.py +++ b/spec/validate.py @@ -23,8 +23,9 @@ CONSUMER_MODELS = ("push", "pull") # Parses owner/repo, lowercased, from a repo's url. # A trailing .git is stripped so it still matches GitHub's own full_name. +# A query character or a fragment character is excluded from both groups too, or a query string or fragment would fold into the repo name instead of failing to match. # A duplicate identity here would let spec/audit.py's fleet membership check silently shadow one entry with the other. -GITHUB_URL_RE = re.compile(r"^https://github\.com/([^/\s]+)/([^/\s]+?)(?:\.git)?/?$") +GITHUB_URL_RE = re.compile(r"^https://github\.com/([^/\s?#]+)/([^/\s?#]+?)(?:\.git)?/?$") # How faithfully a carried unit is checked, per spec/fidelity-model.md, defaulting to presence. FIDELITIES = ("presence", "intent", "verbatim", "interface") # The keys an interface unit's `contract` may carry (kept in sync with files.schema.json). From 50afe891d42ca645732bc00fecdacf5e02bffd14 Mon Sep 17 00:00:00 2001 From: Pieter Viljoen Date: Sat, 22 Aug 2026 09:25:47 -0700 Subject: [PATCH 2/3] Wrap a Comment Under the 100-Character Line Length A Copilot finding caught a comment exceeding pyproject.toml's line-length = 100, added in the previous commit on this branch. Split into two sentences, each within the limit. python3 scripts/prose_lint.py: 0 issues ruff check / ruff format --check: clean --- spec/validate.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/spec/validate.py b/spec/validate.py index 60f5ae132..446f144ee 100755 --- a/spec/validate.py +++ b/spec/validate.py @@ -23,7 +23,8 @@ CONSUMER_MODELS = ("push", "pull") # Parses owner/repo, lowercased, from a repo's url. # A trailing .git is stripped so it still matches GitHub's own full_name. -# A query character or a fragment character is excluded from both groups too, or a query string or fragment would fold into the repo name instead of failing to match. +# A query character or a fragment character is excluded from both groups too. +# Otherwise a query string or fragment folds into the repo name instead of failing to match. # A duplicate identity here would let spec/audit.py's fleet membership check silently shadow one entry with the other. GITHUB_URL_RE = re.compile(r"^https://github\.com/([^/\s?#]+)/([^/\s?#]+?)(?:\.git)?/?$") # How faithfully a carried unit is checked, per spec/fidelity-model.md, defaulting to presence. From 3d1462e7363de5f67e153d6f12ba03822c6d775e Mon Sep 17 00:00:00 2001 From: Pieter Viljoen Date: Sat, 22 Aug 2026 09:31:58 -0700 Subject: [PATCH 3/3] Split a Wrapped Docstring Sentence, Add Owner-Segment URL Cases - qodo caught repo_identity()'s docstring wrapping one sentence across lines 266-267. scripts/prose_lint.py's comment-wrap check only scans '#' comments, not docstrings, so it never caught this. Split into one sentence per line, each within the 100-character limit. - CodeRabbit noted the new query/fragment selftest cases only covered the delimiter appearing after the repo name, not in the owner segment. Added both to prove the first capture group's exclusion too. python3 spec/audit.py --selftest: SELFTEST PASS, including both new cases python3 scripts/prose_lint.py: 0 issues ruff check / ruff format --check: clean mypy spec/audit.py spec/validate.py: no issues --- spec/audit.py | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/spec/audit.py b/spec/audit.py index ff03f754c..7006ba2ec 100755 --- a/spec/audit.py +++ b/spec/audit.py @@ -261,10 +261,10 @@ def owner_repos(owner): def repo_identity(url): """owner/repo, lowercased, parsed from a github.com URL, or None if it does not parse. - The canonical identity two repos are compared by, since a bare repo name collides across - owners and GitHub's own full_name field is already this exact shape (case-preserved). A - trailing .git is stripped so it still resolves to the same identity full_name would, and a - query string or fragment is rejected outright rather than folded into the repo name. + The canonical identity two repos are compared by, since a bare repo name collides across owners. + GitHub's own full_name field is already this exact shape (case-preserved). + A trailing .git is stripped so it still resolves to the same identity full_name would. + A query string or fragment is rejected outright rather than folded into the repo name. spec/validate.py rejects a url that fails to parse here at all, kept in sync with this regex. """ if not isinstance(url, str): @@ -4216,6 +4216,8 @@ def _selftest(): ("https://github.com/owner/Repo?tab=readme", None), ("https://github.com/owner/Repo#readme", None), ("https://github.com/owner/Repo.git?tab=readme", None), + ("https://github.com/owner?tab=readme/Repo", None), + ("https://github.com/owner#readme/Repo", None), ("http://github.com/owner/Repo", None), ("https://gitlab.com/owner/Repo", None), ("not a url", None),