With no --login, both wrappers resolved a login by GUESSING it from the repo host
(get_gitea_login_for_host / get_gitea_login), then looked that name up in
~/.config/tea/config.yml. On git.mosaicstack.dev the guess resolves to
mosaicstack-mos-dt-0, a SHARED account. get_gitea_token_for_login returns the
matching token string with no authentication check anywhere in its body, so it
returns rc=0 for a dead credential and the `|| get_gitea_token` fallback never
fires. The identity-aware resolver was unreachable on this path.
Measured on git.mosaicstack.dev with MOSAIC_GIT_IDENTITY set:
get_gitea_token_for_login <guessed> rc=0, token authenticates HTTP 401
get_gitea_token <host> rc=0, token authenticates HTTP 200
Control: the same endpoint with no credential returns 401.
The dead token is what made this visible; it is not the defect. Had the shared
token been alive, every seat's reviews and comments would have been authored by
the shared account, making Gate-16 author-is-not-reviewer unenforceable across
the fleet. A caller that passes no --login is asking to act as ITSELF, and the
guess answered a question nobody asked.
This removes the guess and its tea lookup from the no---login path in both
wrappers. That path now resolves the acting identity's own credential via
get_gitea_token, which fails loud on a fleet host when no identity resolves.
That refusal is the correct outcome and is deliberately not fallen back from.
Unchanged on purpose:
- get_gitea_token_for_login keeps its behaviour and its 53 assert_token pins in
test-gitea-login-resolution.sh. pr-edit.sh and the explicit --login branches
still use it; --login remains the only way to reach the tea store.
- No in-function verification was added. These wrappers verify every write by
id-plus-author read-back against the credential-derived login, so a revoked
token fails at the write with no misattribution. A GET /user pre-check would
also hard-fail a live token scoped write:repository without read:user, which
returns 403 while being fully comment-capable.
Tests: 28 pass. test-issue-close-fail-closed.sh fails identically on pristine
upstream (byte-identical output) and touches issue-close.sh, which this does not
modify. The wrapper test harness inherits an ambient MOSAIC_GIT_IDENTITY into its
sandbox HOME; tests were run with it unset.
Adversarial review by fargo, who found that reordering alone is a no-op wherever
MOSAIC_GIT_IDENTITY is unset, and that a 401/403 fail-closed pre-check would
reject correctly-scoped live tokens.
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.
fred
jason.woltjeandMos