Merge pull request 'fix(git): accept http/https as one scheme class in comment URL verification (#991)' (#1022) from fix/991-comment-url-scheme-normalise into main

packages/mosaic/framework/tools/git/issue-comment.sh

@@ -254,15 +254,32 @@ from urllib.parse import urlparse
 
 
 def _origin_and_path(url):
-    # Normalize a URL to (scheme, host, effective-port) + comment path. The port
-    # defaults to the scheme's default (80 http / 443 otherwise) so an implicit
-    # port and its explicit default form compare equal.
+    # Normalize a URL to (scheme-class, host, distinguishing-port) + comment path.
+    #
+    # #991: http and https collapse into ONE scheme class ("web"). A Gitea whose
+    # ROOT_URL is configured http:// returns http:// object URLs even when every
+    # client reaches it over https://, so a scheme-strict comparison rejects the
+    # provider's own correct answer about a write that landed — a deterministic
+    # false negative on every comment posted against such a deployment. The
+    # scheme is also not what this check defends: the forgeries it exists to
+    # catch (look-alike host, decoy path prefix, wrong owner/repo/number) all
+    # vary the HOST or the PATH, both of which stay strict below. Any OTHER
+    # scheme (file:, ftp:, javascript:) remains distinguishing and is rejected.
+    #
+    # Port: an implicit port and its own scheme's default compare equal, so
+    # http://h == https://h. An EXPLICIT non-default port still distinguishes,
+    # because a different port is a different service on the same host.
     parsed = urlparse(url or "")
     scheme = (parsed.scheme or "").lower()
     host = (parsed.hostname or "").lower()
-    default_port = 80 if scheme == "http" else 443
-    port = parsed.port if parsed.port is not None else default_port
-    return (scheme, host, port), parsed.path.rstrip("/")
+    if scheme in ("http", "https"):
+        scheme_class = "web"
+        default_port = 80 if scheme == "http" else 443
+        port = None if parsed.port in (None, default_port) else parsed.port
+    else:
+        scheme_class = scheme
+        port = parsed.port
+    return (scheme_class, host, port), parsed.path.rstrip("/")
 
 
 try:

packages/mosaic/framework/tools/git/pr-review.sh

@@ -243,15 +243,35 @@ from urllib.parse import urlparse
 
 
 def _origin_and_path(url):
-    # Normalize a URL to (scheme, host, effective-port) + comment path. The port
-    # defaults to the scheme's default (80 http / 443 otherwise) so an implicit
-    # port and its explicit default form compare equal.
+    # Normalize a URL to (scheme-class, host, distinguishing-port) + comment path.
+    #
+    # #991: http and https collapse into ONE scheme class ("web"). A Gitea whose
+    # ROOT_URL is configured http:// returns http:// object URLs even when every
+    # client reaches it over https://, so a scheme-strict comparison rejects the
+    # provider's own correct answer about a comment that landed — a deterministic
+    # false negative on EVERY review comment posted against such a deployment.
+    # That matters more here than anywhere else: on a host where no seat can
+    # create a review OBJECT, the comment-form review record this path produces
+    # is the only gate-16 evidence available, and this check refuses all of it.
+    # The scheme is also not what the check defends: the forgeries it exists to
+    # catch (look-alike host, decoy path prefix, wrong owner/repo/kind/number)
+    # all vary the HOST or the PATH, both of which stay strict below. Any OTHER
+    # scheme (file:, ftp:, javascript:) remains distinguishing and is rejected.
+    #
+    # Port: an implicit port and its own scheme's default compare equal, so
+    # http://h == https://h. An EXPLICIT non-default port still distinguishes,
+    # because a different port is a different service on the same host.
     parsed = urlparse(url or "")
     scheme = (parsed.scheme or "").lower()
     host = (parsed.hostname or "").lower()
-    default_port = 80 if scheme == "http" else 443
-    port = parsed.port if parsed.port is not None else default_port
-    return (scheme, host, port), parsed.path.rstrip("/")
+    if scheme in ("http", "https"):
+        scheme_class = "web"
+        default_port = 80 if scheme == "http" else 443
+        port = None if parsed.port in (None, default_port) else parsed.port
+    else:
+        scheme_class = scheme
+        port = parsed.port
+    return (scheme_class, host, port), parsed.path.rstrip("/")
 
 
 try:

packages/mosaic/framework/tools/git/test-pr-review-gitea-comment.sh

@@ -439,6 +439,19 @@ elif mode == "comment-url-wrong-repo":
 elif mode == "comment-url-suffix-injection":
     # Prefix-injected: a bare endswith("/<slug>/pulls/123") test would ACCEPT it.
     pr_url = f"{_origin}/deceptive{_slug}/pulls/123"
+elif mode == "comment-url-wrong-port":
+    # #991 bound: an EXPLICIT non-default port is a different service on the same
+    # host. Relaxing http-vs-https must NOT relax this.
+    pr_url = f"{_p.scheme}://{_p.hostname}:8443{_slug}/pulls/123"
+elif mode == "comment-url-non-web-scheme":
+    # #991 bound: ONLY http/https collapse; any other scheme stays distinguishing.
+    pr_url = f"ftp://{_p.netloc}{_slug}/pulls/123"
+elif mode == "comment-url-scheme-downgrade":
+    # #991, and the only URL mode here that must be ACCEPTED. A Gitea whose
+    # ROOT_URL is http:// returns http:// object URLs for a repo reached over
+    # https://. Same host, same path, correct record — a truthful provider
+    # answer about a comment that landed, not a forgery.
+    pr_url = f"http://{_p.netloc}{_slug}/pulls/123"
 elif mode == "comment-mixed-case-slug":
     # #875: EXPECTED_REPO_SLUG is taken verbatim from GITEA_API_BASE and can be
     # mixed-case (e.g. "USC/uconnect"), but Gitea canonicalizes the returned
@@ -893,11 +906,16 @@ fi
 assert_no_temp_leak "review-body-reuse"
 
 # Cases 12-15 (#865 Blocker 3): a PR comment whose id/author/body are all correct
-# but whose provider-returned pull_request_url is forged must FAIL CLOSED.
-# Verification pins the URL's ORIGIN (scheme+host+effective-port) and FULL path
-# (deployment prefix + exact owner/repo + kind + number); a bare endswith/suffix
-# test would wrongly accept the look-alike-host and prefix-injection variants.
-for bad_mode in comment-url-wrong-host comment-url-wrong-owner comment-url-wrong-repo comment-url-suffix-injection; do
+# but whose provider-returned pull_request_url does not belong to this PR must
+# FAIL CLOSED. Verification pins the URL's ORIGIN (scheme-class + host + explicit
+# non-default port) and FULL path (deployment prefix + exact owner/repo + kind +
+# number); a bare endswith/suffix test would wrongly accept the look-alike-host
+# and prefix-injection variants. comment-url-wrong-port and
+# comment-url-non-web-scheme (#991) bound the scheme relaxation from the other
+# side: collapsing http/https must not also collapse a different port or a
+# different scheme family.
+for bad_mode in comment-url-wrong-host comment-url-wrong-owner comment-url-wrong-repo \
+                comment-url-suffix-injection comment-url-wrong-port comment-url-non-web-scheme; do
     if run_review "$bad_mode" comment durable-body; then
         echo "FAIL: forged comment URL ($bad_mode) was accepted" >&2
         cat "$OUTPUT_FILE" >&2
@@ -923,6 +941,19 @@ run_review comment-mixed-case-slug comment durable-body https://git.mosaicstack.
 grep -q 'Added and verified comment on Gitea PR #123' "$OUTPUT_FILE"
 assert_no_temp_leak "comment-mixed-case-slug"
 
+# Case 15c (#991): the deployment's Gitea ROOT_URL is http:// while every client
+# reaches it over https://, so the provider returns an http:// pull_request_url
+# for a comment that is otherwise entirely correct. Same class as 15b — a
+# legitimate provider response, not a spoof — and a scheme-strict compare
+# rejects it on EVERY comment, deterministically. That is not a cosmetic false
+# negative here: on a host where no seat can create a review OBJECT, this
+# comment-form record is the only gate-16 evidence obtainable, and the wrapper
+# refuses all of it while the comment sits durably on the PR. Host, path, owner,
+# repo, kind and number stay strict; only http-vs-https is relaxed.
+run_review comment-url-scheme-downgrade comment durable-body
+grep -q 'Added and verified comment on Gitea PR #123' "$OUTPUT_FILE"
+assert_no_temp_leak "comment-url-scheme-downgrade"
+
 # Case 16 (#865 ITEM 1, current-head TOCTOU): the PR head advances between the
 # pre-submit head read (which pins the review) and the post-verify re-read. The
 # review is genuinely created and verified as pinned to the OLD head, but the