fix(git): accept http/https as one scheme class in comment URL verification (#991)

Conversation 0 Commits 1 Files Changed 4

+147 -26

1 Commits

Author	SHA1	Message	Date
Jason Woltje	d55cbd264e	fix(git): accept http/https as one scheme class in comment URL verification (#991 ) ci/woodpecker/pr/ci Pipeline was successful Details issue-comment.sh and pr-review.sh verify a durable write by pinning the provider-returned object URL's origin and full path. The origin included the SCHEME verbatim. On a Gitea whose ROOT_URL is configured `http://` while every client reaches it over `https://`, the provider returns `http://` object URLs, so the comparison rejects the provider's own truthful answer about a write that LANDED. The failure is deterministic, not intermittent: every comment, every time, on such a deployment. The scheme was never 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 stay strict. `http` and `https` now collapse to one scheme class; any other scheme (file:, ftp:, javascript:) stays distinguishing, and an EXPLICIT non-default port still distinguishes, because a different port is a different service on the same host. Consequences of the bug, both observed: - The wrapper reports failure on a comment that is durably on the issue/PR, and attributes it to #865 ("no durable comment created"). The write landed; the citation is wrong. Reproduced here: the harness's persisted state contains the record while the wrapper exits 1. - pr-review.sh's comment path is worse. On a host where no seat can create a review OBJECT, comment-form is the only gate-16 review record obtainable, and this check refuses all of it. Test gap this closes: every URL fixture in both harnesses was `https://`, and every negative case varied only host or path. The one axis that fails in production had zero coverage — the fixtures encoded the assumption that breaks. Added, in both suites: - scheme-downgrade (http vs https, otherwise correct) — must be ACCEPTED. Fails against the unmodified wrappers, passes against the fixed ones; verified in both directions, and the negative control's captured output is the #865 misattribution above. - explicit non-default port (`:8443`) — must stay REJECTED. - non-web scheme (`ftp://`) — must stay REJECTED. Also fixes test-issue-comment-readback.sh hermeticity (#1007), without which the suite cannot run on any seat that has a per-agent Gitea token: detect-platform's step-0 identity lookup reads ~/.config/mosaic/gitea-tokens/<identity>, outside both XDG_CONFIG_HOME and MOSAIC_CREDENTIALS_FILE, so the suite resolved a PRODUCTION credential and died at HTTP 401 before case 1. Same two-part fix already merged for test-pr-review-gitea-comment.sh in #1006: a sandboxed HOME plus an empty REPO-LOCAL mosaic.gitIdentity to shadow the global. Note the env-var route does NOT work — detect-platform.sh reads `${MOSAIC_GIT_IDENTITY:-}` and `:-` treats set-but-empty identically to unset. The owner-side half of #991 (setting the deployment's Gitea ROOT_URL to https) is not in scope here and is not made unnecessary by this change; this makes the wrappers correct against a deployment that returns either scheme.	2026-07-31 06:22:46 -05:00

Author

SHA1

Message

Date

Jason Woltje

d55cbd264e

fix(git): accept http/https as one scheme class in comment URL verification (#991 )

ci/woodpecker/pr/ci Pipeline was successful

Details

issue-comment.sh and pr-review.sh verify a durable write by pinning the
provider-returned object URL's origin and full path. The origin included the
SCHEME verbatim. On a Gitea whose ROOT_URL is configured `http://` while every
client reaches it over `https://`, the provider returns `http://` object URLs,
so the comparison rejects the provider's own truthful answer about a write that
LANDED. The failure is deterministic, not intermittent: every comment, every
time, on such a deployment.

The scheme was never 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 stay strict. `http` and `https` now collapse to one
scheme class; any other scheme (file:, ftp:, javascript:) stays distinguishing,
and an EXPLICIT non-default port still distinguishes, because a different port
is a different service on the same host.

Consequences of the bug, both observed:

- The wrapper reports failure on a comment that is durably on the issue/PR, and
  attributes it to #865 ("no durable comment created"). The write landed; the
  citation is wrong. Reproduced here: the harness's persisted state contains the
  record while the wrapper exits 1.
- pr-review.sh's comment path is worse. On a host where no seat can create a
  review OBJECT, comment-form is the only gate-16 review record obtainable, and
  this check refuses all of it.

Test gap this closes: every URL fixture in both harnesses was `https://`, and
every negative case varied only host or path. The one axis that fails in
production had zero coverage — the fixtures encoded the assumption that breaks.
Added, in both suites:

- scheme-downgrade (http vs https, otherwise correct) — must be ACCEPTED. Fails
  against the unmodified wrappers, passes against the fixed ones; verified in
  both directions, and the negative control's captured output is the #865
  misattribution above.
- explicit non-default port (`:8443`) — must stay REJECTED.
- non-web scheme (`ftp://`) — must stay REJECTED.

Also fixes test-issue-comment-readback.sh hermeticity (#1007), without which the
suite cannot run on any seat that has a per-agent Gitea token: detect-platform's
step-0 identity lookup reads ~/.config/mosaic/gitea-tokens/<identity>, outside
both XDG_CONFIG_HOME and MOSAIC_CREDENTIALS_FILE, so the suite resolved a
PRODUCTION credential and died at HTTP 401 before case 1. Same two-part fix
already merged for test-pr-review-gitea-comment.sh in #1006: a sandboxed HOME
plus an empty REPO-LOCAL mosaic.gitIdentity to shadow the global. Note the
env-var route does NOT work — detect-platform.sh reads `${MOSAIC_GIT_IDENTITY:-}`
and `:-` treats set-but-empty identically to unset.

The owner-side half of #991 (setting the deployment's Gitea ROOT_URL to https)
is not in scope here and is not made unnecessary by this change; this makes the
wrappers correct against a deployment that returns either scheme.

2026-07-31 06:22:46 -05:00

fix(git): accept http/https as one scheme class in comment URL verification (#991) #1022