diff --git a/packages/mosaic/framework/tools/git/issue-comment.sh b/packages/mosaic/framework/tools/git/issue-comment.sh index 7c17794d..d11cc5e7 100755 --- a/packages/mosaic/framework/tools/git/issue-comment.sh +++ b/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: diff --git a/packages/mosaic/framework/tools/git/pr-review.sh b/packages/mosaic/framework/tools/git/pr-review.sh index ce10fe98..32a4b8eb 100755 --- a/packages/mosaic/framework/tools/git/pr-review.sh +++ b/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: diff --git a/packages/mosaic/framework/tools/git/test-issue-comment-readback.sh b/packages/mosaic/framework/tools/git/test-issue-comment-readback.sh index b0178385..e284cc84 100755 --- a/packages/mosaic/framework/tools/git/test-issue-comment-readback.sh +++ b/packages/mosaic/framework/tools/git/test-issue-comment-readback.sh @@ -41,7 +41,13 @@ # that other host's credential cross-host; # 10. leaves NO temp files behind (POST/GET bodies + metadata) on either the # success or the failure path — nested function-scoped RETURN traps do not -# clobber each other and every scratch file is removed on all exit paths. +# clobber each other and every scratch file is removed on all exit paths; +# 11. (#991) ACCEPTS a record whose provider-returned URL differs from the repo +# remote ONLY in http-vs-https — the standing state on a deployment whose +# Gitea ROOT_URL is misconfigured — while still REJECTING a different +# explicit port and a non-web scheme. Every prior fixture here was https:// +# and every negative varied only host or path, so the one axis that fails +# in production had zero coverage: the fixtures encoded the assumption. set -euo pipefail @@ -61,15 +67,29 @@ STATE_FILE="$WORK_DIR/comments.json" # A dedicated scratch dir the wrapper is pointed at via TMPDIR, so the leak # check can assert every POST/GET body + metadata temp file is cleaned up. TMP_SCRATCH="$WORK_DIR/scratch" +# #1007: a sandboxed HOME. detect-platform.sh's step-0 per-agent identity lookup +# reads ~/.config/mosaic/gitea-tokens/, which is OUTSIDE both +# XDG_CONFIG_HOME and MOSAIC_CREDENTIALS_FILE — so on any seat that has a real +# per-agent token the suite resolves a PRODUCTION credential and dies at the +# authenticated-identity read (HTTP 401) before reaching case 1. Pointing HOME +# at the work dir keeps that lookup inside the sandbox. +HOME_DIR="$WORK_DIR/home" cleanup() { rm -rf "$WORK_DIR" } trap cleanup EXIT -mkdir -p "$REPO_DIR" "$BIN_DIR" "$XDG_DIR" "$TMP_SCRATCH" +mkdir -p "$REPO_DIR" "$BIN_DIR" "$XDG_DIR" "$TMP_SCRATCH" "$HOME_DIR" git -C "$REPO_DIR" init -q git -C "$REPO_DIR" remote add origin https://git.mosaicstack.dev/mosaicstack/stack.git +# #1007: shadow any GLOBAL `mosaic.gitIdentity` with an empty REPO-LOCAL value. +# A repo-local key shadows the global and reads back empty at rc=0, which is the +# only way to make step 0 fall through inside a sandbox. Note the env-var route +# (MOSAIC_GIT_IDENTITY="") does NOT work: detect-platform.sh reads it with +# `${MOSAIC_GIT_IDENTITY:-}`, and `:-` treats set-but-empty identically to +# unset, so setting it empty is silently the same as not setting it at all. +git -C "$REPO_DIR" config mosaic.gitIdentity "" ISSUE_NUMBER=7 REPO_SLUG="mosaicstack/stack" @@ -266,6 +286,19 @@ elif mode == "url-wrong-repo": elif mode == "url-suffix-injection": # Prefix-injected: a bare endswith("//issues/7") test would ACCEPT this. issue_url = f"https://git.mosaicstack.dev/deceptive/{repo}/issues/7" +elif mode == "url-scheme-downgrade": + # #991, and the ONLY one of these modes that must be ACCEPTED. A Gitea whose + # ROOT_URL is http:// returns http:// object URLs for a repo cloned over + # https://. Same host, same path, correct record — the provider is telling + # the truth about a write that landed. + issue_url = f"http://git.mosaicstack.dev/{repo}/issues/7" +elif mode == "url-wrong-port": + # An EXPLICIT non-default port is a different service on the same host and + # must stay distinguishing — collapsing the scheme must not collapse this. + issue_url = f"https://git.mosaicstack.dev:8443/{repo}/issues/7" +elif mode == "url-non-web-scheme": + # Only http/https collapse. Any other scheme stays distinguishing. + issue_url = f"ftp://git.mosaicstack.dev/{repo}/issues/7" record = { "id": new_id, "body": body, @@ -365,6 +398,7 @@ run_comment() { ( cd "$REPO_DIR" PATH="$BIN_DIR:$PATH" \ + HOME="$HOME_DIR" \ TMPDIR="$TMP_SCRATCH" \ XDG_CONFIG_HOME="$XDG_DIR" \ MOSAIC_CREDENTIALS_FILE="$CREDENTIALS_FILE" \ @@ -545,13 +579,17 @@ if grep -q " $ACTING_LOGIN\$" "$AUTH_LOG"; then fi assert_no_temp_leak "cross-host" -# Cases 7-10 (#865 Blocker 3): the created record's id/author/body are all -# correct, but its provider-returned issue_url is forged. Verification pins the -# URL's ORIGIN (scheme+host+effective-port) and its FULL path (deployment prefix -# + exact owner/repo + kind + number), so each forgery must FAIL CLOSED. A bare -# endswith/suffix test would wrongly accept the look-alike-host and -# prefix-injection variants. -for bad_mode in url-wrong-host url-wrong-owner url-wrong-repo url-suffix-injection; do +# Cases 7-12 (#865 Blocker 3): the created record's id/author/body are all +# correct, but its provider-returned issue_url does not belong to this issue on +# this provider/repo. Verification pins the URL's ORIGIN (scheme-class + host + +# explicit non-default port) and its FULL path (deployment prefix + exact +# owner/repo + kind + number), so each must FAIL CLOSED. A bare endswith/suffix +# test would wrongly accept the look-alike-host and prefix-injection variants. +# url-wrong-port and 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 url-wrong-host url-wrong-owner url-wrong-repo url-suffix-injection \ + url-wrong-port url-non-web-scheme; do if run_comment "$bad_mode"; then echo "FAIL: forged comment URL ($bad_mode) was accepted" >&2 cat "$OUTPUT_FILE" >&2 @@ -568,4 +606,19 @@ done # issue_url (already exercised by Case 1's fresh-success), so the tightened check # is not rejecting genuine writes. +# Case 13 (#991): the deployment's ROOT_URL is http:// while the repo remote is +# https://, so the provider returns an http:// issue_url for a record that is +# otherwise entirely correct. This is not a forgery — it is the provider's own +# truthful answer about a write that landed — and a scheme-strict comparison +# rejects it deterministically, on EVERY comment, converting a successful write +# into a reported failure. It must be ACCEPTED. Host, path, owner, repo, kind +# and number all remain strict; only the http/https distinction is relaxed. +run_comment url-scheme-downgrade +grep -q 'Added and verified comment on Gitea issue #7 (comment ID 1)' "$OUTPUT_FILE" +# The write really happened and was read back by exact id — this case passes +# through the same POST/GET chain as case 1, not around it. +grep -q "^POST $API_BASE/issues/7/comments$" "$CURL_LOG" +grep -q "^GET $API_BASE/issues/comments/1$" "$CURL_LOG" +assert_no_temp_leak "url-scheme-downgrade" + echo "issue-comment.sh REST create + exact-id read-back regression passed" diff --git a/packages/mosaic/framework/tools/git/test-pr-review-gitea-comment.sh b/packages/mosaic/framework/tools/git/test-pr-review-gitea-comment.sh index 588ca0bc..3ac5a5b1 100644 --- a/packages/mosaic/framework/tools/git/test-pr-review-gitea-comment.sh +++ b/packages/mosaic/framework/tools/git/test-pr-review-gitea-comment.sh @@ -436,6 +436,19 @@ elif mode == "comment-url-wrong-repo": elif mode == "comment-url-suffix-injection": # Prefix-injected: a bare endswith("//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 @@ -890,11 +903,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 @@ -920,6 +938,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