From 6a9b00f96924c8e29db11ab2dbfd526e39fa965e Mon Sep 17 00:00:00 2001 From: fred Date: Thu, 20 Aug 2026 22:50:11 -0500 Subject: [PATCH] fix(git-wrappers): remove the guessed-login credential path from pr-review and issue-comment 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 rc=0, token authenticates HTTP 401 get_gitea_token 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. --- .../framework/tools/git/issue-comment.sh | 27 ++++---- .../mosaic/framework/tools/git/pr-review.sh | 64 ++++++++++--------- 2 files changed, 49 insertions(+), 42 deletions(-) diff --git a/packages/mosaic/framework/tools/git/issue-comment.sh b/packages/mosaic/framework/tools/git/issue-comment.sh index d11cc5e7..49b4b504 100755 --- a/packages/mosaic/framework/tools/git/issue-comment.sh +++ b/packages/mosaic/framework/tools/git/issue-comment.sh @@ -102,9 +102,17 @@ gitea_resolve_api_for_login() { return 1 } else - GITEA_API_TOKEN=$(get_gitea_token_for_login "$effective_login" "$host") \ - || GITEA_API_TOKEN=$(get_gitea_token "$host") || { - echo "Error: Gitea token not found for login '$effective_login' (comment write/read-back)" >&2 + # NO --login: the acting credential is this identity's own token and there + # is deliberately no tea-config fallback. get_gitea_token_for_login matches + # by login NAME and performs no authentication check, and with no --login + # that name was a HOST GUESS resolving to a shared account. A live shared + # token would therefore have authored every seat's comment as that + # account, making Gate-16 author-is-not-reviewer unenforceable fleet-wide; + # a dead one is only what made the defect visible. get_gitea_token fails + # loud on a fleet host when no identity resolves, and that refusal is the + # correct outcome, not a case to fall back from. + GITEA_API_TOKEN=$(get_gitea_token "$host") || { + echo "Error: no Gitea credential resolved for the acting identity on host '$host' (comment write/read-back). Set MOSAIC_GIT_IDENTITY=, or pass --login to use a named tea credential." >&2 return 1 } fi @@ -335,15 +343,12 @@ if [[ "$PLATFORM" == "github" ]]; then gh issue comment "$ISSUE_NUMBER" --body "$COMMENT" echo "Added comment to GitHub issue #$ISSUE_NUMBER" elif [[ "$PLATFORM" == "gitea" ]]; then - # Resolve the login this comment should be attributed to: the --login - # override when given, otherwise the detected default for this repo's host. - # A --login override always wins. Otherwise name this repo host's login only - # as a best effort: the login name merely selects a per-login token, and - # gitea_resolve_api_for_login falls back to the host credential - # (get_gitea_token) when no tea login is named, so the default credential - # still resolves even when the host tea has no matching login entry. + # A --login override selects a NAMED tea credential and is the only way to + # reach the tea store. With no --login there is deliberately no guess: the + # comment is attributed to this identity's own credential, resolved by + # gitea_resolve_api_for_login. The guess this replaced named a SHARED + # account, selecting an identity the caller never asked to act as. EFFECTIVE_LOGIN="$LOGIN_OVERRIDE" - [[ -n "$EFFECTIVE_LOGIN" ]] || EFFECTIVE_LOGIN=$(get_gitea_login 2>/dev/null || true) # Bind the REST endpoint + token to the effective login, then derive the # acting identity from that SAME credential (GET /user). The write below and diff --git a/packages/mosaic/framework/tools/git/pr-review.sh b/packages/mosaic/framework/tools/git/pr-review.sh index 32a4b8eb..b131f385 100755 --- a/packages/mosaic/framework/tools/git/pr-review.sh +++ b/packages/mosaic/framework/tools/git/pr-review.sh @@ -12,7 +12,7 @@ # concurrent record cannot masquerade as this write and a no-op fails closed. # # --login override: the default login is resolved from the local tea login list -# for this repo's host (get_gitea_login_for_host). Pass --login to +# for this repo's host from the acting identity's own credential. Pass --login to # override it for this invocation only. The REST write, the /user identity read, # and every read-back are ALL performed with the token of the EFFECTIVE login, # so the write and its verification bind to the same identity. @@ -372,9 +372,17 @@ gitea_resolve_api_for_login() { return 1 } else - GITEA_API_TOKEN=$(get_gitea_token_for_login "$effective_login" "$host") \ - || GITEA_API_TOKEN=$(get_gitea_token "$host") || { - echo "Error: Gitea token not found for login '$effective_login' (review write/read-back)" >&2 + # NO --login: the acting credential is this identity's own token and there + # is deliberately no tea-config fallback. get_gitea_token_for_login matches + # by login NAME and performs no authentication check, and with no --login + # that name was a HOST GUESS resolving to a shared account. A live shared + # token would therefore have authored every seat's review as that + # account, making Gate-16 author-is-not-reviewer unenforceable fleet-wide; + # a dead one is only what made the defect visible. get_gitea_token fails + # loud on a fleet host when no identity resolves, and that refusal is the + # correct outcome, not a case to fall back from. + GITEA_API_TOKEN=$(get_gitea_token "$host") || { + echo "Error: no Gitea credential resolved for the acting identity on host '$host' (review write/read-back). Set MOSAIC_GIT_IDENTITY=, or pass --login to use a named tea credential." >&2 return 1 } fi @@ -698,7 +706,7 @@ if [[ "$PLATFORM" == "github" ]]; then elif [[ "$PLATFORM" == "gitea" ]]; then case $ACTION in approve) - # Best-effort host for the tea-login GUESS only (gitea_resolve_api_for_login + # Best-effort host for credential resolution only (gitea_resolve_api_for_login # below re-derives the real host from HOST_OVERRIDE/remote independently and # is authoritative). Prefer an explicit -H/--host; otherwise best-effort # git-remote inference, tolerating its ABSENCE (a bare `get_remote_host` here @@ -706,15 +714,13 @@ elif [[ "$PLATFORM" == "gitea" ]]; then # SILENTLY — exit 1, zero output — even though -r/-H are exactly the flags # that support running with no usable origin at all). host="${HOST_OVERRIDE:-$(get_remote_host 2>/dev/null || true)}" - # A --login override always wins. Otherwise name this host's login - # only as a best effort: the login name merely selects a per-login - # token, and gitea_resolve_api_for_login falls back to the host - # credential (get_gitea_token) when no tea login is named — so a host - # tea's login list need not enumerate exotic (e.g. ported) hosts for - # the default credential to resolve. The single resolved token is - # then used for the write, the /user identity, and the read-back. + # A --login override selects a NAMED tea credential and is the only + # way to reach the tea store. With no --login there is deliberately no + # guess: gitea_resolve_api_for_login resolves this identity's own token. + # The guess this replaced named a SHARED account, selecting an identity + # the caller never asked to act as. The single resolved token is then + # used for the write, the /user identity, and the read-back. EFFECTIVE_LOGIN="$LOGIN_OVERRIDE" - [[ -n "$EFFECTIVE_LOGIN" ]] || EFFECTIVE_LOGIN=$(get_gitea_login_for_host "$host" 2>/dev/null || true) # Bind the REST endpoint + token to the effective login, then derive # the acting identity from that SAME credential so the review submit # and its read-back verify against the identity that performed them. @@ -735,7 +741,7 @@ elif [[ "$PLATFORM" == "gitea" ]]; then echo "Error: Comment required for request-changes" exit 1 fi - # Best-effort host for the tea-login GUESS only (gitea_resolve_api_for_login + # Best-effort host for credential resolution only (gitea_resolve_api_for_login # below re-derives the real host from HOST_OVERRIDE/remote independently and # is authoritative). Prefer an explicit -H/--host; otherwise best-effort # git-remote inference, tolerating its ABSENCE (a bare `get_remote_host` here @@ -743,15 +749,13 @@ elif [[ "$PLATFORM" == "gitea" ]]; then # SILENTLY — exit 1, zero output — even though -r/-H are exactly the flags # that support running with no usable origin at all). host="${HOST_OVERRIDE:-$(get_remote_host 2>/dev/null || true)}" - # A --login override always wins. Otherwise name this host's login - # only as a best effort: the login name merely selects a per-login - # token, and gitea_resolve_api_for_login falls back to the host - # credential (get_gitea_token) when no tea login is named — so a host - # tea's login list need not enumerate exotic (e.g. ported) hosts for - # the default credential to resolve. The single resolved token is - # then used for the write, the /user identity, and the read-back. + # A --login override selects a NAMED tea credential and is the only + # way to reach the tea store. With no --login there is deliberately no + # guess: gitea_resolve_api_for_login resolves this identity's own token. + # The guess this replaced named a SHARED account, selecting an identity + # the caller never asked to act as. The single resolved token is then + # used for the write, the /user identity, and the read-back. EFFECTIVE_LOGIN="$LOGIN_OVERRIDE" - [[ -n "$EFFECTIVE_LOGIN" ]] || EFFECTIVE_LOGIN=$(get_gitea_login_for_host "$host" 2>/dev/null || true) gitea_resolve_api_for_login "$EFFECTIVE_LOGIN" "${LOGIN_OVERRIDE:+explicit}" || exit 1 ACTING_LOGIN=$(gitea_authenticated_login) || exit 1 head_sha=$(gitea_pr_head_sha "$PR_NUMBER") || exit 1 @@ -766,7 +770,7 @@ elif [[ "$PLATFORM" == "gitea" ]]; then echo "Error: Comment required" exit 1 fi - # Best-effort host for the tea-login GUESS only (gitea_resolve_api_for_login + # Best-effort host for credential resolution only (gitea_resolve_api_for_login # below re-derives the real host from HOST_OVERRIDE/remote independently and # is authoritative). Prefer an explicit -H/--host; otherwise best-effort # git-remote inference, tolerating its ABSENCE (a bare `get_remote_host` here @@ -774,15 +778,13 @@ elif [[ "$PLATFORM" == "gitea" ]]; then # SILENTLY — exit 1, zero output — even though -r/-H are exactly the flags # that support running with no usable origin at all). host="${HOST_OVERRIDE:-$(get_remote_host 2>/dev/null || true)}" - # A --login override always wins. Otherwise name this host's login - # only as a best effort: the login name merely selects a per-login - # token, and gitea_resolve_api_for_login falls back to the host - # credential (get_gitea_token) when no tea login is named — so a host - # tea's login list need not enumerate exotic (e.g. ported) hosts for - # the default credential to resolve. The single resolved token is - # then used for the write, the /user identity, and the read-back. + # A --login override selects a NAMED tea credential and is the only + # way to reach the tea store. With no --login there is deliberately no + # guess: gitea_resolve_api_for_login resolves this identity's own token. + # The guess this replaced named a SHARED account, selecting an identity + # the caller never asked to act as. The single resolved token is then + # used for the write, the /user identity, and the read-back. EFFECTIVE_LOGIN="$LOGIN_OVERRIDE" - [[ -n "$EFFECTIVE_LOGIN" ]] || EFFECTIVE_LOGIN=$(get_gitea_login_for_host "$host" 2>/dev/null || true) gitea_resolve_api_for_login "$EFFECTIVE_LOGIN" "${LOGIN_OVERRIDE:+explicit}" || exit 1 ACTING_LOGIN=$(gitea_authenticated_login) || exit 1 comment_id=$(gitea_create_comment_verified "$PR_NUMBER" "$COMMENT" "$ACTING_LOGIN") || {