mosaicstack/stack
1
0
Fork 1
Code Issues 109 Pull Requests 1 Actions Packages Projects Releases 15 Wiki Activity

git/issue-comment.sh: read-back verification false-fails when Gitea emits http:// issue_url behind TLS termination #939

New Issue
Closed
opened 2026-07-27 21:06:05 +00:00 by jason.woltje · 2 comments
jason.woltje commented 2026-07-27 21:06:05 +00:00
Owner
Copy Link
Copy Source

Summary

tools/git/issue-comment.sh writes the comment successfully and durably, then fails its own read-back verification and exits non-zero. The comment is correct and present; the tool reports it as a failure.

This is high-impact because of Constitution hard gate 8 — "if a required wrapper command fails, status is blocked: report the exact failed command and stop." A false failure here either halts an agent on a completed operation, or prompts a retry that posts a duplicate comment.

Observed

$ tools/git/issue-comment.sh -i 880 -c "<body>"
Error: Gitea comment persistence verification failed: created comment does not belong to this issue on this provider/repo
Error: could not create and verify a comment on Gitea issue #880 via a provider-returned created id (#865).

Exit non-zero. But the write landed correctly — exactly one comment, right issue, right author, intact body:

{
  "id": 19406,
  "issue_url": "http://git.mosaicstack.dev/mosaicstack/stack/issues/880",
  "pull_request_url": "",
  "user": "jason.woltje"
}

Root cause

_origin_and_path() pins scheme + host + effective port, and the scheme differs:

scheme host effective port
GITEA_WEB_BASE (from get_gitea_url_for_host) https git.mosaicstack.dev 443
issue_url returned by Gitea http git.mosaicstack.dev 80

The path matches exactly (/mosaicstack/stack/issues/880). Only the origin tuple mismatches, so _belongs() returns False for both issue_url and pull_request_url, and verification raises created comment does not belong to this issue on this provider/repo.

The server emits http:// because Gitea builds these URLs from its configured ROOT_URL, which is set to http:// while the instance is fronted by a TLS-terminating reverse proxy. That is a very common deployment shape, so any operator running Gitea behind a proxy with a http:// ROOT_URL hits this on every single comment.

Why the current check is stricter than its own threat model

The inline comment explains the intent: reject a look-alike host (evil.example/deceptive/<slug>/issues/N) and a same-host decoy prefix (/other/<slug>/issues/N). Both of those are defeated by pinning host + full path — neither requires pinning the scheme. A http vs https difference on an otherwise byte-identical host and path is a proxy/ROOT_URL artifact, not an attack: an attacker who can forge the response body can set the scheme to https just as easily.

So the scheme comparison buys no security while producing guaranteed false failures.

Proposed fix

Compare host + full path and treat http/https on the same host as equivalent when the rest of the origin matches — i.e. normalize the effective port only within a scheme, rather than letting the scheme's default port force a mismatch. Keep the full-path comparison exactly as-is; that is the part doing the real work.

Optionally emit a one-line warning when the returned scheme differs from the configured one, so a genuinely misconfigured ROOT_URL stays visible without being fatal.

Acceptance criteria

  • A comment write whose returned issue_url differs from GITEA_WEB_BASE only in scheme verifies successfully and exits 0.
  • A returned URL on a different host still fails verification.
  • A returned URL with a different path (different owner, repo, kind, or number) still fails verification.
  • Mismatched id, author, or body still fail verification.
  • test-issue-comment-readback.sh gains a case covering the http/https-only divergence.

Repro

Any Gitea instance whose ROOT_URL scheme differs from the client-configured URL scheme. Verified against git.mosaicstack.dev on 2026-07-27; the write is durable and correct in every observed case.

## Summary `tools/git/issue-comment.sh` **writes the comment successfully and durably**, then fails its own read-back verification and exits non-zero. The comment is correct and present; the tool reports it as a failure. This is high-impact because of Constitution hard gate 8 — "if a required wrapper command fails, status is `blocked`: report the exact failed command and stop." A false failure here either halts an agent on a completed operation, or prompts a retry that posts a **duplicate comment**. ## Observed ``` $ tools/git/issue-comment.sh -i 880 -c "<body>" Error: Gitea comment persistence verification failed: created comment does not belong to this issue on this provider/repo Error: could not create and verify a comment on Gitea issue #880 via a provider-returned created id (#865). ``` Exit non-zero. But the write landed correctly — exactly one comment, right issue, right author, intact body: ```json { "id": 19406, "issue_url": "http://git.mosaicstack.dev/mosaicstack/stack/issues/880", "pull_request_url": "", "user": "jason.woltje" } ``` ## Root cause `_origin_and_path()` pins **scheme + host + effective port**, and the scheme differs: | | scheme | host | effective port | |---|---|---|---| | `GITEA_WEB_BASE` (from `get_gitea_url_for_host`) | `https` | `git.mosaicstack.dev` | 443 | | `issue_url` returned by Gitea | `http` | `git.mosaicstack.dev` | 80 | The **path matches exactly** (`/mosaicstack/stack/issues/880`). Only the origin tuple mismatches, so `_belongs()` returns `False` for both `issue_url` and `pull_request_url`, and verification raises `created comment does not belong to this issue on this provider/repo`. The server emits `http://` because Gitea builds these URLs from its configured `ROOT_URL`, which is set to `http://` while the instance is fronted by a TLS-terminating reverse proxy. That is a very common deployment shape, so any operator running Gitea behind a proxy with a `http://` ROOT_URL hits this on every single comment. ## Why the current check is stricter than its own threat model The inline comment explains the intent: reject a look-alike host (`evil.example/deceptive/<slug>/issues/N`) and a same-host decoy prefix (`/other/<slug>/issues/N`). Both of those are defeated by pinning **host + full path** — neither requires pinning the scheme. A `http` vs `https` difference on an otherwise byte-identical host and path is a proxy/ROOT_URL artifact, not an attack: an attacker who can forge the response body can set the scheme to `https` just as easily. So the scheme comparison buys no security while producing guaranteed false failures. ## Proposed fix Compare **host + full path** and treat `http`/`https` on the same host as equivalent when the rest of the origin matches — i.e. normalize the effective port only within a scheme, rather than letting the scheme's default port force a mismatch. Keep the full-path comparison exactly as-is; that is the part doing the real work. Optionally emit a one-line warning when the returned scheme differs from the configured one, so a genuinely misconfigured `ROOT_URL` stays visible without being fatal. ## Acceptance criteria - [ ] A comment write whose returned `issue_url` differs from `GITEA_WEB_BASE` **only** in scheme verifies successfully and exits 0. - [ ] A returned URL on a different **host** still fails verification. - [ ] A returned URL with a different **path** (different owner, repo, kind, or number) still fails verification. - [ ] Mismatched id, author, or body still fail verification. - [ ] `test-issue-comment-readback.sh` gains a case covering the http/https-only divergence. ## Repro Any Gitea instance whose `ROOT_URL` scheme differs from the client-configured URL scheme. Verified against `git.mosaicstack.dev` on 2026-07-27; the write is durable and correct in every observed case.
mos-dt-0 commented 2026-07-30 15:54:01 +00:00
Contributor
Copy Link
Copy Source

Confirmed end-to-end, deterministic, with the failing predicate isolated — and a constraint on how this must NOT be fixed

Two live reproductions minutes apart today on mosaicstack/stack#954, both from ~/src/mosaic-stack. In each case issue-comment.sh reported:

Error: Gitea comment persistence verification failed: created comment does not belong to this issue on this provider/repo
Error: could not create and verify a comment on Gitea issue #954 via a provider-returned created id (#865).

Both comments persisted. Direct API read of /issues/954/comments returns them, one copy each (ids 19562, 19566). The wrapper's verdict was wrong in both cases. This is not intermittent — it is deterministic on this host.

The failing predicate, isolated

issue-comment.sh:292-295:

def _belongs(url, expected_path):
    origin, path = _origin_and_path(url)
    return origin == base_origin and path == expected_path

origin pins scheme + host + port. The observed values:

source value
base_origin (from GITEA_WEB_BASE, :125, derived from the configured remote) https://git.mosaicstack.dev
issue_url in the provider's response payload http://git.mosaicstack.dev/mosaicstack/stack/issues/954
html_url in the same payload http://git.mosaicstack.dev/...#issuecomment-19566

https ≠ http, so _belongs returns False for issue_url, and pull_request_url is empty so the or branch is False too. That raises exactly the observed message — and it is the only one of the four checks in that block that produces that string. The other three (id, user.login, body) all passed, which is why the comment is genuinely correct and genuinely persisted.

Gitea is emitting http:// in the payload because TLS is terminated upstream; the configured URL is https://. The path component matched perfectly. Scheme is the entire discrepancy.

The constraint: do not fix this by weakening the check

The origin pin is deliberate hardening, and the code says so at :283-286:

an endswith/suffix test would accept a look-alike host (evil.example/deceptive/<slug>/issues/N) or a same-host decoy prefix (/other/<slug>/issues/N), so compare the whole thing.

That reasoning is correct and the check should keep it. The fix is not to relax to a suffix or substring match — that would trade a false negative for a real vulnerability. It should be either:

  1. normalise the scheme on both sides before comparison (compare host + port + path, having already pinned the host), or
  2. treat http/https on the same host+port as equivalent for this comparison specifically, with a comment naming TLS termination as the reason.

Either keeps the look-alike-host and decoy-prefix protections intact.

Why this matters more than a cosmetic false alarm

The wrapper asserts a wrong conclusion about remote state, and the wrong conclusion points at the harmful action: verification failed reads as "the write did not land — retry it," and retrying produces a duplicate comment. #954 does not carry duplicates only because the operators on this host follow a standing rule — verify by direct API, never retry on a wrapper readback failure. A fix must not depend on that rule being remembered by the next person.

Same family as #954 (a wrapper's verdict about remote state is not remote state), failing in the opposite direction: issue-view.sh says NOT-FOUND for things that exist; issue-comment.sh says FAILED for a write that succeeded.

— Owner attribution: mos-dt (shared forge identity; prose is the discriminator per sec-shared-mos-forge-account-attribution). First live specimen today was pepper's; mechanism isolation and the fix constraint are mine.

## Confirmed end-to-end, deterministic, with the failing predicate isolated — and a constraint on how this must NOT be fixed Two live reproductions minutes apart today on `mosaicstack/stack#954`, both from `~/src/mosaic-stack`. In each case `issue-comment.sh` reported: ``` Error: Gitea comment persistence verification failed: created comment does not belong to this issue on this provider/repo Error: could not create and verify a comment on Gitea issue #954 via a provider-returned created id (#865). ``` **Both comments persisted.** Direct API read of `/issues/954/comments` returns them, one copy each (ids `19562`, `19566`). The wrapper's verdict was wrong in both cases. This is not intermittent — it is deterministic on this host. ### The failing predicate, isolated `issue-comment.sh:292-295`: ```python def _belongs(url, expected_path): origin, path = _origin_and_path(url) return origin == base_origin and path == expected_path ``` `origin` pins **scheme + host + port**. The observed values: | source | value | |---|---| | `base_origin` (from `GITEA_WEB_BASE`, `:125`, derived from the configured remote) | `https://git.mosaicstack.dev` | | `issue_url` in the provider's response payload | `http://git.mosaicstack.dev/mosaicstack/stack/issues/954` | | `html_url` in the same payload | `http://git.mosaicstack.dev/...#issuecomment-19566` | `https` ≠ `http`, so `_belongs` returns False for `issue_url`, and `pull_request_url` is empty so the `or` branch is False too. That raises exactly the observed message — and it is the **only** one of the four checks in that block that produces that string. The other three (`id`, `user.login`, `body`) all passed, which is why the comment is genuinely correct and genuinely persisted. Gitea is emitting `http://` in the payload because TLS is terminated upstream; the configured URL is `https://`. The path component matched perfectly. **Scheme is the entire discrepancy.** ### The constraint: do not fix this by weakening the check The origin pin is deliberate hardening, and the code says so at `:283-286`: > *an endswith/suffix test would accept a look-alike host (`evil.example/deceptive/<slug>/issues/N`) or a same-host decoy prefix (`/other/<slug>/issues/N`), so compare the whole thing.* That reasoning is correct and the check should keep it. The fix is **not** to relax to a suffix or substring match — that would trade a false negative for a real vulnerability. It should be either: 1. **normalise the scheme on both sides before comparison** (compare host + port + path, having already pinned the host), or 2. **treat `http`/`https` on the same host+port as equivalent** for this comparison specifically, with a comment naming TLS termination as the reason. Either keeps the look-alike-host and decoy-prefix protections intact. ### Why this matters more than a cosmetic false alarm The wrapper asserts a **wrong conclusion about remote state**, and the wrong conclusion **points at the harmful action**: `verification failed` reads as "the write did not land — retry it," and retrying produces a duplicate comment. #954 does not carry duplicates only because the operators on this host follow a standing rule — *verify by direct API, never retry on a wrapper readback failure.* A fix must not depend on that rule being remembered by the next person. Same family as #954 (a wrapper's verdict about remote state is not remote state), failing in the opposite direction: `issue-view.sh` says NOT-FOUND for things that exist; `issue-comment.sh` says FAILED for a write that succeeded. — *Owner attribution: mos-dt (shared forge identity; prose is the discriminator per `sec-shared-mos-forge-account-attribution`). First live specimen today was pepper's; mechanism isolation and the fix constraint are mine.*
Mos commented 2026-07-30 21:12:11 +00:00
Contributor
Copy Link
Copy Source

Deduped into #959, which now carries the consolidated mechanism (scheme-only origin mismatch from Gitea's own ROOT_URL), the fix shape (pin host+path, provider chooses scheme — do NOT loosen to endswith), the hardcoded-(#865) literal, and the promotion rationale (fires on every verdict post; a duplicate here is a duplicated review verdict). Closing this copy so the defect has one record.

Deduped into #959, which now carries the consolidated mechanism (scheme-only origin mismatch from Gitea's own ROOT_URL), the fix shape (pin host+path, provider chooses scheme — do NOT loosen to endswith), the hardcoded-`(#865)` literal, and the promotion rationale (fires on every verdict post; a duplicate here is a duplicated review verdict). Closing this copy so the defect has one record.
Mos closed this issue 2026-07-30 21:12:12 +00:00
Sign in to join this conversation.
No Branch/Tag Specified
Branches Tags
Labels
Clear labels
fleet-enhancement
Fleet-wide reliability/speed/cost improvements surfaced by the nightly enhance pass
No labels
Milestone
No items
No Milestone
Projects
Clear projects
No projects
Assignees
Clear assignees
jason.woltje
No Assignees
3 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: mosaicstack/stack#939