From 6168f9ac86e52c28441b6563cddb94230ffdb303 Mon Sep 17 00:00:00 2001 From: Hermes Agent Date: Tue, 21 Jul 2026 20:07:00 -0500 Subject: [PATCH] fix(git-tools): fail closed on unresolvable explicit --login override (#865) gitea_resolve_api_for_login silently fell back to the host-default identity whenever the named login could not be resolved for ANY reason -- including when the name came from an EXPLICIT --login override. A caller passing a dedicated per-role credential could thus have its write attributed to the shared default identity while being told it succeeded as requested. Thread an "override was explicit" signal into gitea_resolve_api_for_login (second param, "explicit" when LOGIN_OVERRIDE is non-empty). When the override is explicit and that login's token cannot be resolved, FAIL CLOSED (return 1, clear error naming the login, no host-default fallback). The best-effort host-default fallback now applies ONLY on the no-override default path. Applied symmetrically to issue-comment.sh and all three pr-review.sh dispatch sites (approve / request-changes / comment). Tests: both scripts' write flows now assert credential attribution via a token->identity seam in the curl stub -- (a) resolvable --login override drives the entire write/read-back chain under THAT login's token, nothing under the default; (b) unresolvable --login override fails closed (nonzero, no success line, no write, no default-identity request); (c) no-override default path still succeeds under the host-default best-effort credential. Co-Authored-By: Claude Opus 4.8 --- .../framework/tools/git/issue-comment.sh | 34 ++++-- .../mosaic/framework/tools/git/pr-review.sh | 32 ++++-- .../tools/git/test-issue-comment-readback.sh | 102 ++++++++++++++++- .../tools/git/test-pr-review-gitea-comment.sh | 107 +++++++++++++++++- 4 files changed, 245 insertions(+), 30 deletions(-) diff --git a/packages/mosaic/framework/tools/git/issue-comment.sh b/packages/mosaic/framework/tools/git/issue-comment.sh index ce08d023..f225844b 100755 --- a/packages/mosaic/framework/tools/git/issue-comment.sh +++ b/packages/mosaic/framework/tools/git/issue-comment.sh @@ -85,17 +85,29 @@ detect_platform >/dev/null # and read-back token are the same identity by construction (this is the # credential-ordering fix: a --login override is no longer written under one # credential and verified under a different default one). Falls back to the -# host-scoped credential only when the login has no token in tea's own config. -# Returns non-zero (clear stderr) on any resolution failure. +# host-scoped credential ONLY when NO --login override was supplied (the +# best-effort default path). When $2 is "explicit" the login came from a +# caller-supplied --login: that exact login's token MUST resolve, and we FAIL +# CLOSED rather than silently downgrading the write to the host default +# identity — otherwise a caller relying on a dedicated per-role credential would +# be told the write succeeded as requested while it was attributed to the shared +# default. Returns non-zero (clear stderr) on any resolution failure. gitea_resolve_api_for_login() { - local effective_login="$1" host configured_url repo + local effective_login="$1" override_explicit="${2:-}" host configured_url repo host=$(get_remote_host) - GITEA_API_TOKEN=$(get_gitea_token_for_login "$effective_login") \ - || GITEA_API_TOKEN=$(get_gitea_token "$host") || { - echo "Error: Gitea token not found for login '$effective_login' (comment write/read-back)" >&2 - return 1 - } + if [[ -n "$override_explicit" ]]; then + GITEA_API_TOKEN=$(get_gitea_token_for_login "$effective_login") || { + echo "Error: could not resolve a Gitea token for --login '$effective_login'; refusing to fall back to the host default identity (comment write/read-back)" >&2 + return 1 + } + else + GITEA_API_TOKEN=$(get_gitea_token_for_login "$effective_login") \ + || GITEA_API_TOKEN=$(get_gitea_token "$host") || { + echo "Error: Gitea token not found for login '$effective_login' (comment write/read-back)" >&2 + return 1 + } + fi configured_url=$(get_gitea_url_for_host "$host") || { echo "Error: Configured Gitea URL not found for comment read-back verification" >&2 return 1 @@ -379,8 +391,10 @@ elif [[ "$PLATFORM" == "gitea" ]]; then # Bind the REST endpoint + token to the effective login, then derive the # acting identity from that SAME credential (GET /user). The write below and # its read-back both use this credential, so the write is verified against - # the identity that actually performed it. - gitea_resolve_api_for_login "$EFFECTIVE_LOGIN" || exit 1 + # the identity that actually performed it. Passing "explicit" when --login + # was supplied forbids the host-default fallback: an unresolvable explicit + # override fails closed instead of writing under the default identity. + 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 "$ISSUE_NUMBER" "$COMMENT" "$ACTING_LOGIN") || { diff --git a/packages/mosaic/framework/tools/git/pr-review.sh b/packages/mosaic/framework/tools/git/pr-review.sh index ca2315d1..555c6e6a 100755 --- a/packages/mosaic/framework/tools/git/pr-review.sh +++ b/packages/mosaic/framework/tools/git/pr-review.sh @@ -193,17 +193,27 @@ PY # write token and read-back token are the same identity by construction. This # is the credential-ordering fix: a --login override is no longer submitted # under one credential and verified under a different default one. Falls back to -# the host-scoped credential only when the login has no token in tea's config. -# Returns non-zero (clear stderr) on any resolution failure. +# the host-scoped credential ONLY when NO --login override was supplied (the +# best-effort default path). When $2 is "explicit" the login came from a +# caller-supplied --login: that exact login's token MUST resolve, and we FAIL +# CLOSED rather than silently downgrading the review/comment to the host default +# identity. Returns non-zero (clear stderr) on any resolution failure. gitea_resolve_api_for_login() { - local effective_login="$1" host configured_url repo + local effective_login="$1" override_explicit="${2:-}" host configured_url repo host=$(get_remote_host) - GITEA_API_TOKEN=$(get_gitea_token_for_login "$effective_login") \ - || GITEA_API_TOKEN=$(get_gitea_token "$host") || { - echo "Error: Gitea token not found for login '$effective_login' (review write/read-back)" >&2 - return 1 - } + if [[ -n "$override_explicit" ]]; then + GITEA_API_TOKEN=$(get_gitea_token_for_login "$effective_login") || { + echo "Error: could not resolve a Gitea token for --login '$effective_login'; refusing to fall back to the host default identity (review write/read-back)" >&2 + return 1 + } + else + GITEA_API_TOKEN=$(get_gitea_token_for_login "$effective_login") \ + || GITEA_API_TOKEN=$(get_gitea_token "$host") || { + echo "Error: Gitea token not found for login '$effective_login' (review write/read-back)" >&2 + return 1 + } + fi configured_url=$(get_gitea_url_for_host "$host") || { echo "Error: Configured Gitea URL not found for review read-back verification" >&2 return 1 @@ -552,7 +562,7 @@ elif [[ "$PLATFORM" == "gitea" ]]; then # 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. - gitea_resolve_api_for_login "$EFFECTIVE_LOGIN" || exit 1 + 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 # The review body (if any) travels with the review itself in the REST @@ -579,7 +589,7 @@ elif [[ "$PLATFORM" == "gitea" ]]; then # 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" || exit 1 + 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 review_id=$(gitea_submit_review_verified "$PR_NUMBER" "REQUEST_CHANGES" "$COMMENT" "$ACTING_LOGIN" "$head_sha") || { @@ -603,7 +613,7 @@ elif [[ "$PLATFORM" == "gitea" ]]; then # 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" || exit 1 + 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") || { echo "Error: could not create and verify a comment on Gitea PR #$PR_NUMBER via a provider-returned created id (#865)." >&2 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 d32662ea..6eb3bdfe 100755 --- a/packages/mosaic/framework/tools/git/test-issue-comment-readback.sh +++ b/packages/mosaic/framework/tools/git/test-issue-comment-readback.sh @@ -27,7 +27,14 @@ # 5. fails closed when the created record is not authored by the acting # identity; # 6. enumerates the created id in the issue's FULLY PAGINATED comment list, -# finding it even when it lands beyond page 1. +# finding it even when it lands beyond page 1; +# 7. with a RESOLVABLE --login override, performs the write, the /user identity +# lookup, and the read-back ALL under THAT login's token/identity — never +# the host default; +# 8. with an UNRESOLVABLE --login override, FAILS CLOSED (nonzero, no write, no +# success line) instead of silently downgrading to the host default +# identity — the token seam maps each bearer token to the identity it +# authenticates as, so a misattributed write is caught. set -euo pipefail @@ -38,6 +45,7 @@ BIN_DIR="$WORK_DIR/bin" XDG_DIR="$WORK_DIR/xdg" TEA_LOG="$WORK_DIR/tea.log" CURL_LOG="$WORK_DIR/curl.log" +AUTH_LOG="$WORK_DIR/auth.log" OUTPUT_FILE="$WORK_DIR/output.log" CREDENTIALS_FILE="$WORK_DIR/credentials.json" STATE_FILE="$WORK_DIR/comments.json" @@ -58,6 +66,27 @@ API_ROOT="https://git.mosaicstack.dev/api/v1" BODY='durable "note" -- marker' ACTING_LOGIN="primary-reviewer" FOREIGN_LOGIN="other-writer" +# A dedicated per-role --login override identity, with its own token stored in +# tea's config (exactly the author-not-equal-reviewer hardening path). +OVERRIDE_LOGIN="delegated-reviewer" +DEFAULT_TOKEN="test-only-placeholder" +OVERRIDE_TOKEN="override-token-placeholder" + +# tea config: the override login has its own token here (as tea itself stores +# per-login tokens). The default login name ("mosaicstack") is deliberately NOT +# present, so the no-override default path resolves via the host credential +# fallback while an explicit --login must resolve from this file or fail closed. +mkdir -p "$XDG_DIR/tea" +OVERRIDE_LOGIN="$OVERRIDE_LOGIN" OVERRIDE_TOKEN="$OVERRIDE_TOKEN" python3 - "$XDG_DIR/tea/config.yml" <<'PY' +import os +import sys + +with open(sys.argv[1], "w", encoding="utf-8") as handle: + handle.write("logins:\n") + handle.write(f" - name: {os.environ['OVERRIDE_LOGIN']}\n") + handle.write(" url: https://git.mosaicstack.dev\n") + handle.write(f" token: {os.environ['OVERRIDE_TOKEN']}\n") +PY CONFIGURED_GITEA_URL="https://git.mosaicstack.dev" python3 - "$CREDENTIALS_FILE" <<'PY' import json @@ -106,10 +135,14 @@ output_file="" method="GET" url="" data="" +auth_token="" while [[ $# -gt 0 ]]; do case "$1" in -o) output_file="$2"; shift 2 ;; - -w|-H) shift 2 ;; + -H) + [[ "$2" == Authorization:* ]] && auth_token="${2##* }" + shift 2 ;; + -w) shift 2 ;; -X) method="$2"; shift 2 ;; -d|--data) data="$2"; shift 2 ;; -s|-S|-sS) shift ;; @@ -123,6 +156,17 @@ query="${url#*\?}" [[ "$query" == "$url" ]] && query="" printf '%s %s\n' "$method" "$url" >> "$ISSUE_COMMENT_CURL_LOG" +# Map the presented bearer token to the identity it authenticates as — the same +# derivation Gitea's own /user does. The wrapper's write, /user lookup, and +# read-back must all carry the SAME token, so the acting identity recorded here +# reveals which credential actually performed the request. +acting_identity="" +case "$auth_token" in + "$ISSUE_COMMENT_DEFAULT_TOKEN") acting_identity="$ISSUE_COMMENT_ACTING_LOGIN" ;; + "$ISSUE_COMMENT_OVERRIDE_TOKEN") acting_identity="$ISSUE_COMMENT_OVERRIDE_LOGIN" ;; +esac +printf '%s %s %s\n' "$method" "$path" "${acting_identity:-}" >> "$ISSUE_COMMENT_AUTH_LOG" + write_response() { local status="$1" body="$2" [[ -n "$output_file" ]] || exit 96 @@ -131,14 +175,15 @@ write_response() { } if [[ "$method" == "GET" && "$path" == "$ISSUE_COMMENT_API_ROOT/user" ]]; then - write_response 200 "$(ISSUE_COMMENT_LOGIN="$ISSUE_COMMENT_ACTING_LOGIN" python3 - <<'PY' + [[ -n "$acting_identity" ]] || { write_response 401 '{"message":"unauthenticated"}'; exit 0; } + write_response 200 "$(ISSUE_COMMENT_LOGIN="$acting_identity" python3 - <<'PY' import json import os print(json.dumps({"login": os.environ["ISSUE_COMMENT_LOGIN"]})) PY )" elif [[ "$method" == "POST" && "$path" == "$ISSUE_COMMENT_API_BASE/issues/7/comments" ]]; then - result=$(ISSUE_COMMENT_DATA="$data" python3 - <<'PY' + result=$(ISSUE_COMMENT_ACTING_LOGIN="${acting_identity:-$ISSUE_COMMENT_ACTING_LOGIN}" ISSUE_COMMENT_DATA="$data" python3 - <<'PY' import json import os @@ -261,8 +306,10 @@ PY run_comment() { local mode="$1" + shift : > "$TEA_LOG" : > "$CURL_LOG" + : > "$AUTH_LOG" : > "$OUTPUT_FILE" seed_state "$mode" ( @@ -272,14 +319,18 @@ run_comment() { MOSAIC_CREDENTIALS_FILE="$CREDENTIALS_FILE" \ ISSUE_COMMENT_TEA_LOG="$TEA_LOG" \ ISSUE_COMMENT_CURL_LOG="$CURL_LOG" \ + ISSUE_COMMENT_AUTH_LOG="$AUTH_LOG" \ ISSUE_COMMENT_STATE="$STATE_FILE" \ ISSUE_COMMENT_TEST_MODE="$mode" \ ISSUE_COMMENT_ACTING_LOGIN="$ACTING_LOGIN" \ ISSUE_COMMENT_FOREIGN_LOGIN="$FOREIGN_LOGIN" \ + ISSUE_COMMENT_OVERRIDE_LOGIN="$OVERRIDE_LOGIN" \ + ISSUE_COMMENT_DEFAULT_TOKEN="$DEFAULT_TOKEN" \ + ISSUE_COMMENT_OVERRIDE_TOKEN="$OVERRIDE_TOKEN" \ ISSUE_COMMENT_REPO_SLUG="$REPO_SLUG" \ ISSUE_COMMENT_API_BASE="$API_BASE" \ ISSUE_COMMENT_API_ROOT="$API_ROOT" \ - "$SCRIPT_DIR/issue-comment.sh" -i "$ISSUE_NUMBER" -c "$BODY" + "$SCRIPT_DIR/issue-comment.sh" -i "$ISSUE_NUMBER" -c "$BODY" "$@" ) > "$OUTPUT_FILE" 2>&1 } @@ -299,6 +350,10 @@ grep -q "^GET $API_BASE/issues/comments/51$" "$CURL_LOG" grep -q "^GET $API_ROOT/user$" "$CURL_LOG" # Enumeration paginated beyond page 1 to find the created comment. grep -q "^GET $API_BASE/issues/7/comments?limit=[0-9]*&page=2$" "$CURL_LOG" +# Default path (no --login): the host credential fallback resolves, and the +# write is performed AND self-verified under the host-default acting identity. +grep -q "^POST $API_BASE/issues/7/comments $ACTING_LOGIN$" "$AUTH_LOG" +grep -q "^GET $API_BASE/issues/comments/51 $ACTING_LOGIN$" "$AUTH_LOG" # Case 2: a no-op write with a concurrent SAME-IDENTITY, same-body comment # already present must FAIL CLOSED — the closed concurrency window. @@ -328,4 +383,41 @@ if grep -q 'Added and verified comment' "$OUTPUT_FILE"; then exit 1 fi +# Case 4: a RESOLVABLE --login override — the write, the /user identity lookup, +# and the read-back must ALL be performed under THAT login's token/identity, not +# the host default. The override login has id 1 (empty seed). +run_comment override-success --login "$OVERRIDE_LOGIN" +grep -q 'Added and verified comment on Gitea issue #7 (comment ID 1)' "$OUTPUT_FILE" +grep -q "^GET $API_ROOT/user $OVERRIDE_LOGIN$" "$AUTH_LOG" +grep -q "^POST $API_BASE/issues/7/comments $OVERRIDE_LOGIN$" "$AUTH_LOG" +grep -q "^GET $API_BASE/issues/comments/1 $OVERRIDE_LOGIN$" "$AUTH_LOG" +# The host-default identity must NOT have performed ANY request in this run. +if grep -q " $ACTING_LOGIN\$" "$AUTH_LOG"; then + echo "FAIL: an explicit --login override request was performed under the host default identity" >&2 + cat "$AUTH_LOG" >&2 + exit 1 +fi + +# Case 5: an UNRESOLVABLE --login override (name absent from tea config) must +# FAIL CLOSED — no silent downgrade to the host default identity: nonzero exit, +# no success line, and NO write performed. +if run_comment override-unresolvable --login "nonexistent-typo-login"; then + echo "FAIL: unresolvable --login override did not fail closed" >&2 + cat "$OUTPUT_FILE" >&2 + exit 1 +fi +if grep -q 'Added and verified comment' "$OUTPUT_FILE"; then + echo "FAIL: unresolvable --login override reported success" >&2 + exit 1 +fi +if grep -q "^POST $API_BASE/issues/7/comments" "$CURL_LOG"; then + echo "FAIL: unresolvable --login override still performed a write" >&2 + exit 1 +fi +# And it must not have silently fallen back to the host default identity. +if grep -q " $ACTING_LOGIN\$" "$AUTH_LOG"; then + echo "FAIL: unresolvable --login override fell back to the host default identity" >&2 + exit 1 +fi + 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 25f36cb3..3108235e 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 @@ -18,6 +18,14 @@ # the read-back reads that same state. There is no independently fabricated # record for the wrapper to "find" — verification passes only when the POST # genuinely created the record the read-back retrieves. +# +# #865 Round-4: the curl stub also maps the presented bearer token to the +# identity it authenticates as and logs it per request, so tests can prove +# credential attribution. An explicit --login override must drive the entire +# write→read-back chain under THAT login's token (resolvable case) or FAIL +# CLOSED (unresolvable case) — never silently downgrade to the host-default +# identity. The host-default best-effort fallback is reserved for the +# no-override default path. set -euo pipefail @@ -32,6 +40,7 @@ COMMENTS_FILE="$STATE_DIR/comments.json" SUBMIT_PAYLOAD_FILE="$STATE_DIR/review_payload.json" TEA_LOG="$WORK_DIR/tea.log" CURL_LOG="$WORK_DIR/curl.log" +AUTH_LOG="$WORK_DIR/auth.log" OUTPUT_FILE="$WORK_DIR/output.log" CREDENTIALS_FILE="$WORK_DIR/credentials.json" @@ -43,11 +52,32 @@ trap cleanup EXIT ACTING_LOGIN="review-bot" FOREIGN_LOGIN="other-writer" HEAD_SHA="HEADSHA_FEEDFACE" +# A dedicated per-role --login override identity with its own token in tea's +# config (the author-not-equal-reviewer hardening path). +OVERRIDE_LOGIN="primary-reviewer" +DEFAULT_TOKEN="test-only-placeholder" +OVERRIDE_TOKEN="override-token-placeholder" mkdir -p "$REPO_DIR" "$BIN_DIR" "$XDG_DIR" "$STATE_DIR" git -C "$REPO_DIR" init -q git -C "$REPO_DIR" remote add origin https://git.mosaicstack.dev/mosaicstack/stack.git +# tea config: the override login carries its own token here. The default login +# name ("mosaicstack") is deliberately absent, so the no-override default path +# resolves via the host credential fallback while an explicit --login must +# resolve from this file or fail closed. +mkdir -p "$XDG_DIR/tea" +OVERRIDE_LOGIN="$OVERRIDE_LOGIN" OVERRIDE_TOKEN="$OVERRIDE_TOKEN" python3 - "$XDG_DIR/tea/config.yml" <<'PY' +import os +import sys + +with open(sys.argv[1], "w", encoding="utf-8") as handle: + handle.write("logins:\n") + handle.write(f" - name: {os.environ['OVERRIDE_LOGIN']}\n") + handle.write(" url: https://git.mosaicstack.dev\n") + handle.write(f" token: {os.environ['OVERRIDE_TOKEN']}\n") +PY + write_credentials() { local configured_url="$1" CONFIGURED_GITEA_URL="$configured_url" python3 - "$CREDENTIALS_FILE" <<'PY' @@ -96,10 +126,14 @@ output_file="" method="GET" payload="" url="" +auth_token="" while [[ $# -gt 0 ]]; do case "$1" in -o) output_file="$2"; shift 2 ;; - -w|-H) shift 2 ;; + -H) + [[ "$2" == Authorization:* ]] && auth_token="${2##* }" + shift 2 ;; + -w) shift 2 ;; -X) method="$2"; shift 2 ;; -d|--data) payload="$2"; shift 2 ;; -s|-S|-sS) shift ;; @@ -113,6 +147,17 @@ query="${url#*\?}" [[ "$query" == "$url" ]] && query="" printf '%s %s\n' "$method" "$url" >> "$PR_REVIEW_CURL_LOG" +# Map the presented bearer token to the identity it authenticates as (as Gitea's +# /user does). The write, /user lookup, and read-back must all carry the SAME +# token, so the identity logged here reveals which credential performed each +# request — proving an explicit --login override is honored, not downgraded. +acting_identity="" +case "$auth_token" in + "$PR_REVIEW_DEFAULT_TOKEN") acting_identity="$PR_REVIEW_ACTING_LOGIN" ;; + "$PR_REVIEW_OVERRIDE_TOKEN") acting_identity="$PR_REVIEW_OVERRIDE_LOGIN" ;; +esac +printf '%s %s %s\n' "$method" "$path" "${acting_identity:-}" >> "$PR_REVIEW_AUTH_LOG" + write_response() { local status="$1" body="$2" [[ -n "$output_file" ]] || exit 96 @@ -129,7 +174,8 @@ emit() { mode="${PR_REVIEW_TEST_MODE:-}" if [[ "$method" == "GET" && "$path" == "$PR_REVIEW_API_ROOT/user" ]]; then - write_response 200 "$(PR_REVIEW_LOGIN="$PR_REVIEW_ACTING_LOGIN" python3 - <<'PY' + [[ -n "$acting_identity" ]] || { write_response 401 '{"message":"unauthenticated"}'; exit 0; } + write_response 200 "$(PR_REVIEW_LOGIN="$acting_identity" python3 - <<'PY' import json import os print(json.dumps({"login": os.environ["PR_REVIEW_LOGIN"]})) @@ -144,7 +190,7 @@ PY )" elif [[ "$method" == "POST" && "$path" == "$PR_REVIEW_EXPECTED_API_BASE/pulls/123/reviews" ]]; then printf '%s' "$payload" > "$PR_REVIEW_SUBMIT_PAYLOAD" - emit "$(PR_REVIEW_PAYLOAD="$payload" python3 - <<'PY' + emit "$(PR_REVIEW_ACTING_LOGIN="${acting_identity:-$PR_REVIEW_ACTING_LOGIN}" PR_REVIEW_PAYLOAD="$payload" python3 - <<'PY' import json import os @@ -321,12 +367,14 @@ run_review() { local configured_url="${4:-https://git.mosaicstack.dev}" local remote_url="${5:-https://git.mosaicstack.dev/mosaicstack/stack.git}" local expected_repo="${6:-mosaicstack/stack}" + local login_override="${7:-}" local expected_api_base="${configured_url%/}/api/v1/repos/$expected_repo" local expected_api_root="${configured_url%/}/api/v1" git -C "$REPO_DIR" remote set-url origin "$remote_url" write_credentials "$configured_url" : > "$TEA_LOG" : > "$CURL_LOG" + : > "$AUTH_LOG" : > "$OUTPUT_FILE" seed_state "$mode" ( @@ -337,6 +385,7 @@ run_review() { PR_REVIEW_TEA_LOG="$TEA_LOG" \ PR_REVIEW_LOGIN_URL="${configured_url%/}" \ PR_REVIEW_CURL_LOG="$CURL_LOG" \ + PR_REVIEW_AUTH_LOG="$AUTH_LOG" \ PR_REVIEW_REVIEWS="$REVIEWS_FILE" \ PR_REVIEW_COMMENTS="$COMMENTS_FILE" \ PR_REVIEW_SUBMIT_PAYLOAD="$SUBMIT_PAYLOAD_FILE" \ @@ -347,7 +396,10 @@ run_review() { PR_REVIEW_HEAD_SHA="$HEAD_SHA" \ PR_REVIEW_ACTING_LOGIN="$ACTING_LOGIN" \ PR_REVIEW_FOREIGN_LOGIN="$FOREIGN_LOGIN" \ - "$SCRIPT_DIR/pr-review.sh" -n 123 -a "$action" ${comment:+-c "$comment"} + PR_REVIEW_OVERRIDE_LOGIN="$OVERRIDE_LOGIN" \ + PR_REVIEW_DEFAULT_TOKEN="$DEFAULT_TOKEN" \ + PR_REVIEW_OVERRIDE_TOKEN="$OVERRIDE_TOKEN" \ + "$SCRIPT_DIR/pr-review.sh" -n 123 -a "$action" ${comment:+-c "$comment"} ${login_override:+--login "$login_override"} ) > "$OUTPUT_FILE" 2>&1 } @@ -370,6 +422,10 @@ grep -q '^GET https://git.mosaicstack.dev/api/v1/repos/mosaicstack/stack/pulls/1 grep -q '^POST https://git.mosaicstack.dev/api/v1/repos/mosaicstack/stack/pulls/123/reviews$' "$CURL_LOG" grep -q '^GET https://git.mosaicstack.dev/api/v1/repos/mosaicstack/stack/pulls/123/reviews/101$' "$CURL_LOG" grep -q '^GET https://git.mosaicstack.dev/api/v1/repos/mosaicstack/stack/pulls/123/reviews?limit=[0-9]*&page=1$' "$CURL_LOG" +# No-override default path: the write, /user lookup, and read-back all resolve +# via the host-default credential and authenticate as the acting identity. +grep -q "^POST https://git.mosaicstack.dev/api/v1/repos/mosaicstack/stack/pulls/123/reviews $ACTING_LOGIN\$" "$AUTH_LOG" +grep -q "^GET https://git.mosaicstack.dev/api/v1/user $ACTING_LOGIN\$" "$AUTH_LOG" assert_no_tea_write # The submitted review payload carries the event and the PR head commit_id. PR_REVIEW_HEAD_SHA="$HEAD_SHA" python3 - "$SUBMIT_PAYLOAD_FILE" <<'PY' @@ -521,4 +577,47 @@ if grep -q 'Added and verified comment' "$OUTPUT_FILE"; then exit 1 fi +# Case 8 (#865 Round-4): a RESOLVABLE explicit --login override must attribute +# the entire write→read-back chain to THAT login's token/identity, never the +# host-default identity. The override login carries its own token in the tea +# config, so /user, the review POST, and the exact-id read-back all authenticate +# as the override identity — and NOTHING is performed under the default identity. +run_review override-success approve "" https://git.mosaicstack.dev \ + https://git.mosaicstack.dev/mosaicstack/stack.git mosaicstack/stack "$OVERRIDE_LOGIN" +grep -q 'Approved and verified Gitea PR #123 (review ID 101)' "$OUTPUT_FILE" +grep -q "^GET https://git.mosaicstack.dev/api/v1/user $OVERRIDE_LOGIN\$" "$AUTH_LOG" +grep -q "^POST https://git.mosaicstack.dev/api/v1/repos/mosaicstack/stack/pulls/123/reviews $OVERRIDE_LOGIN\$" "$AUTH_LOG" +grep -q "^GET https://git.mosaicstack.dev/api/v1/repos/mosaicstack/stack/pulls/123/reviews/101 $OVERRIDE_LOGIN\$" "$AUTH_LOG" +if grep -q " $ACTING_LOGIN\$" "$AUTH_LOG"; then + echo "FAIL: an explicit --login override was silently downgraded to the host-default identity" >&2 + cat "$AUTH_LOG" >&2 + exit 1 +fi +assert_no_tea_write + +# Case 9 (#865 Round-4): an UNRESOLVABLE explicit --login override (a name absent +# from the tea config) must FAIL CLOSED — nonzero exit, no success line, no review +# POST, and above all NO request performed under the host-default identity. The +# host-default best-effort fallback is reserved for the no-override path only. +if run_review override-unresolvable approve "" https://git.mosaicstack.dev \ + https://git.mosaicstack.dev/mosaicstack/stack.git mosaicstack/stack "nonexistent-typo-login"; then + echo "FAIL: an unresolvable --login override was not rejected (silently used the host default)" >&2 + cat "$OUTPUT_FILE" >&2 + exit 1 +fi +if grep -q 'Approved and verified' "$OUTPUT_FILE"; then + echo "FAIL: unresolvable --login override reported success" >&2 + exit 1 +fi +if grep -q '/pulls/123/reviews ' "$AUTH_LOG" && grep -qE '^POST .*/pulls/123/reviews ' "$AUTH_LOG"; then + echo "FAIL: unresolvable --login override performed a review POST" >&2 + cat "$AUTH_LOG" >&2 + exit 1 +fi +if grep -q " $ACTING_LOGIN\$" "$AUTH_LOG"; then + echo "FAIL: unresolvable --login override fell back to the host-default identity" >&2 + cat "$AUTH_LOG" >&2 + exit 1 +fi + echo "pr-review.sh REST review + comment create/read-back regression passed"