fix(git-tools): fail closed on unresolvable explicit --login override (#865)
Some checks failed
ci/woodpecker/pr/ci Pipeline failed

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 <noreply@anthropic.com>
This commit is contained in:
Hermes Agent
2026-07-21 20:07:00 -05:00
parent 9384f0bc0a
commit 6168f9ac86
4 changed files with 245 additions and 30 deletions

View File

@@ -85,17 +85,29 @@ detect_platform >/dev/null
# and read-back token are the same identity by construction (this is the # 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-ordering fix: a --login override is no longer written under one
# credential and verified under a different default one). Falls back to the # 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. # host-scoped credential ONLY when NO --login override was supplied (the
# Returns non-zero (clear stderr) on any resolution failure. # 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() { 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) host=$(get_remote_host)
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_for_login "$effective_login") \
|| GITEA_API_TOKEN=$(get_gitea_token "$host") || { || GITEA_API_TOKEN=$(get_gitea_token "$host") || {
echo "Error: Gitea token not found for login '$effective_login' (comment write/read-back)" >&2 echo "Error: Gitea token not found for login '$effective_login' (comment write/read-back)" >&2
return 1 return 1
} }
fi
configured_url=$(get_gitea_url_for_host "$host") || { configured_url=$(get_gitea_url_for_host "$host") || {
echo "Error: Configured Gitea URL not found for comment read-back verification" >&2 echo "Error: Configured Gitea URL not found for comment read-back verification" >&2
return 1 return 1
@@ -379,8 +391,10 @@ elif [[ "$PLATFORM" == "gitea" ]]; then
# Bind the REST endpoint + token to the effective login, then derive the # Bind the REST endpoint + token to the effective login, then derive the
# acting identity from that SAME credential (GET /user). The write below and # 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 # its read-back both use this credential, so the write is verified against
# the identity that actually performed it. # the identity that actually performed it. Passing "explicit" when --login
gitea_resolve_api_for_login "$EFFECTIVE_LOGIN" || exit 1 # 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 ACTING_LOGIN=$(gitea_authenticated_login) || exit 1
comment_id=$(gitea_create_comment_verified "$ISSUE_NUMBER" "$COMMENT" "$ACTING_LOGIN") || { comment_id=$(gitea_create_comment_verified "$ISSUE_NUMBER" "$COMMENT" "$ACTING_LOGIN") || {

View File

@@ -193,17 +193,27 @@ PY
# write token and read-back token are the same identity by construction. This # 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 # 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 # 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. # the host-scoped credential ONLY when NO --login override was supplied (the
# Returns non-zero (clear stderr) on any resolution failure. # 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() { 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) host=$(get_remote_host)
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_for_login "$effective_login") \
|| GITEA_API_TOKEN=$(get_gitea_token "$host") || { || GITEA_API_TOKEN=$(get_gitea_token "$host") || {
echo "Error: Gitea token not found for login '$effective_login' (review write/read-back)" >&2 echo "Error: Gitea token not found for login '$effective_login' (review write/read-back)" >&2
return 1 return 1
} }
fi
configured_url=$(get_gitea_url_for_host "$host") || { configured_url=$(get_gitea_url_for_host "$host") || {
echo "Error: Configured Gitea URL not found for review read-back verification" >&2 echo "Error: Configured Gitea URL not found for review read-back verification" >&2
return 1 return 1
@@ -552,7 +562,7 @@ elif [[ "$PLATFORM" == "gitea" ]]; then
# Bind the REST endpoint + token to the effective login, then derive # Bind the REST endpoint + token to the effective login, then derive
# the acting identity from that SAME credential so the review submit # the acting identity from that SAME credential so the review submit
# and its read-back verify against the identity that performed them. # 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 ACTING_LOGIN=$(gitea_authenticated_login) || exit 1
head_sha=$(gitea_pr_head_sha "$PR_NUMBER") || 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 # 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. # then used for the write, the /user identity, and the read-back.
EFFECTIVE_LOGIN="$LOGIN_OVERRIDE" EFFECTIVE_LOGIN="$LOGIN_OVERRIDE"
[[ -n "$EFFECTIVE_LOGIN" ]] || EFFECTIVE_LOGIN=$(get_gitea_login_for_host "$host" 2>/dev/null || true) [[ -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 ACTING_LOGIN=$(gitea_authenticated_login) || exit 1
head_sha=$(gitea_pr_head_sha "$PR_NUMBER") || 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") || { 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. # then used for the write, the /user identity, and the read-back.
EFFECTIVE_LOGIN="$LOGIN_OVERRIDE" EFFECTIVE_LOGIN="$LOGIN_OVERRIDE"
[[ -n "$EFFECTIVE_LOGIN" ]] || EFFECTIVE_LOGIN=$(get_gitea_login_for_host "$host" 2>/dev/null || true) [[ -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 ACTING_LOGIN=$(gitea_authenticated_login) || exit 1
comment_id=$(gitea_create_comment_verified "$PR_NUMBER" "$COMMENT" "$ACTING_LOGIN") || { 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 echo "Error: could not create and verify a comment on Gitea PR #$PR_NUMBER via a provider-returned created id (#865)." >&2

View File

@@ -27,7 +27,14 @@
# 5. fails closed when the created record is not authored by the acting # 5. fails closed when the created record is not authored by the acting
# identity; # identity;
# 6. enumerates the created id in the issue's FULLY PAGINATED comment list, # 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 set -euo pipefail
@@ -38,6 +45,7 @@ BIN_DIR="$WORK_DIR/bin"
XDG_DIR="$WORK_DIR/xdg" XDG_DIR="$WORK_DIR/xdg"
TEA_LOG="$WORK_DIR/tea.log" TEA_LOG="$WORK_DIR/tea.log"
CURL_LOG="$WORK_DIR/curl.log" CURL_LOG="$WORK_DIR/curl.log"
AUTH_LOG="$WORK_DIR/auth.log"
OUTPUT_FILE="$WORK_DIR/output.log" OUTPUT_FILE="$WORK_DIR/output.log"
CREDENTIALS_FILE="$WORK_DIR/credentials.json" CREDENTIALS_FILE="$WORK_DIR/credentials.json"
STATE_FILE="$WORK_DIR/comments.json" STATE_FILE="$WORK_DIR/comments.json"
@@ -58,6 +66,27 @@ API_ROOT="https://git.mosaicstack.dev/api/v1"
BODY='durable "note" -- marker' BODY='durable "note" -- marker'
ACTING_LOGIN="primary-reviewer" ACTING_LOGIN="primary-reviewer"
FOREIGN_LOGIN="other-writer" 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' CONFIGURED_GITEA_URL="https://git.mosaicstack.dev" python3 - "$CREDENTIALS_FILE" <<'PY'
import json import json
@@ -106,10 +135,14 @@ output_file=""
method="GET" method="GET"
url="" url=""
data="" data=""
auth_token=""
while [[ $# -gt 0 ]]; do while [[ $# -gt 0 ]]; do
case "$1" in case "$1" in
-o) output_file="$2"; shift 2 ;; -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 ;; -X) method="$2"; shift 2 ;;
-d|--data) data="$2"; shift 2 ;; -d|--data) data="$2"; shift 2 ;;
-s|-S|-sS) shift ;; -s|-S|-sS) shift ;;
@@ -123,6 +156,17 @@ query="${url#*\?}"
[[ "$query" == "$url" ]] && query="" [[ "$query" == "$url" ]] && query=""
printf '%s %s\n' "$method" "$url" >> "$ISSUE_COMMENT_CURL_LOG" 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:-<unauthenticated>}" >> "$ISSUE_COMMENT_AUTH_LOG"
write_response() { write_response() {
local status="$1" body="$2" local status="$1" body="$2"
[[ -n "$output_file" ]] || exit 96 [[ -n "$output_file" ]] || exit 96
@@ -131,14 +175,15 @@ write_response() {
} }
if [[ "$method" == "GET" && "$path" == "$ISSUE_COMMENT_API_ROOT/user" ]]; then 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 json
import os import os
print(json.dumps({"login": os.environ["ISSUE_COMMENT_LOGIN"]})) print(json.dumps({"login": os.environ["ISSUE_COMMENT_LOGIN"]}))
PY PY
)" )"
elif [[ "$method" == "POST" && "$path" == "$ISSUE_COMMENT_API_BASE/issues/7/comments" ]]; then 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 json
import os import os
@@ -261,8 +306,10 @@ PY
run_comment() { run_comment() {
local mode="$1" local mode="$1"
shift
: > "$TEA_LOG" : > "$TEA_LOG"
: > "$CURL_LOG" : > "$CURL_LOG"
: > "$AUTH_LOG"
: > "$OUTPUT_FILE" : > "$OUTPUT_FILE"
seed_state "$mode" seed_state "$mode"
( (
@@ -272,14 +319,18 @@ run_comment() {
MOSAIC_CREDENTIALS_FILE="$CREDENTIALS_FILE" \ MOSAIC_CREDENTIALS_FILE="$CREDENTIALS_FILE" \
ISSUE_COMMENT_TEA_LOG="$TEA_LOG" \ ISSUE_COMMENT_TEA_LOG="$TEA_LOG" \
ISSUE_COMMENT_CURL_LOG="$CURL_LOG" \ ISSUE_COMMENT_CURL_LOG="$CURL_LOG" \
ISSUE_COMMENT_AUTH_LOG="$AUTH_LOG" \
ISSUE_COMMENT_STATE="$STATE_FILE" \ ISSUE_COMMENT_STATE="$STATE_FILE" \
ISSUE_COMMENT_TEST_MODE="$mode" \ ISSUE_COMMENT_TEST_MODE="$mode" \
ISSUE_COMMENT_ACTING_LOGIN="$ACTING_LOGIN" \ ISSUE_COMMENT_ACTING_LOGIN="$ACTING_LOGIN" \
ISSUE_COMMENT_FOREIGN_LOGIN="$FOREIGN_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_REPO_SLUG="$REPO_SLUG" \
ISSUE_COMMENT_API_BASE="$API_BASE" \ ISSUE_COMMENT_API_BASE="$API_BASE" \
ISSUE_COMMENT_API_ROOT="$API_ROOT" \ 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 ) > "$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" grep -q "^GET $API_ROOT/user$" "$CURL_LOG"
# Enumeration paginated beyond page 1 to find the created comment. # 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" 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 # Case 2: a no-op write with a concurrent SAME-IDENTITY, same-body comment
# already present must FAIL CLOSED — the closed concurrency window. # 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 exit 1
fi 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" echo "issue-comment.sh REST create + exact-id read-back regression passed"

View File

@@ -18,6 +18,14 @@
# the read-back reads that same state. There is no independently fabricated # the read-back reads that same state. There is no independently fabricated
# record for the wrapper to "find" — verification passes only when the POST # record for the wrapper to "find" — verification passes only when the POST
# genuinely created the record the read-back retrieves. # 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 set -euo pipefail
@@ -32,6 +40,7 @@ COMMENTS_FILE="$STATE_DIR/comments.json"
SUBMIT_PAYLOAD_FILE="$STATE_DIR/review_payload.json" SUBMIT_PAYLOAD_FILE="$STATE_DIR/review_payload.json"
TEA_LOG="$WORK_DIR/tea.log" TEA_LOG="$WORK_DIR/tea.log"
CURL_LOG="$WORK_DIR/curl.log" CURL_LOG="$WORK_DIR/curl.log"
AUTH_LOG="$WORK_DIR/auth.log"
OUTPUT_FILE="$WORK_DIR/output.log" OUTPUT_FILE="$WORK_DIR/output.log"
CREDENTIALS_FILE="$WORK_DIR/credentials.json" CREDENTIALS_FILE="$WORK_DIR/credentials.json"
@@ -43,11 +52,32 @@ trap cleanup EXIT
ACTING_LOGIN="review-bot" ACTING_LOGIN="review-bot"
FOREIGN_LOGIN="other-writer" FOREIGN_LOGIN="other-writer"
HEAD_SHA="HEADSHA_FEEDFACE" 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" mkdir -p "$REPO_DIR" "$BIN_DIR" "$XDG_DIR" "$STATE_DIR"
git -C "$REPO_DIR" init -q git -C "$REPO_DIR" init -q
git -C "$REPO_DIR" remote add origin https://git.mosaicstack.dev/mosaicstack/stack.git 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() { write_credentials() {
local configured_url="$1" local configured_url="$1"
CONFIGURED_GITEA_URL="$configured_url" python3 - "$CREDENTIALS_FILE" <<'PY' CONFIGURED_GITEA_URL="$configured_url" python3 - "$CREDENTIALS_FILE" <<'PY'
@@ -96,10 +126,14 @@ output_file=""
method="GET" method="GET"
payload="" payload=""
url="" url=""
auth_token=""
while [[ $# -gt 0 ]]; do while [[ $# -gt 0 ]]; do
case "$1" in case "$1" in
-o) output_file="$2"; shift 2 ;; -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 ;; -X) method="$2"; shift 2 ;;
-d|--data) payload="$2"; shift 2 ;; -d|--data) payload="$2"; shift 2 ;;
-s|-S|-sS) shift ;; -s|-S|-sS) shift ;;
@@ -113,6 +147,17 @@ query="${url#*\?}"
[[ "$query" == "$url" ]] && query="" [[ "$query" == "$url" ]] && query=""
printf '%s %s\n' "$method" "$url" >> "$PR_REVIEW_CURL_LOG" 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:-<unauthenticated>}" >> "$PR_REVIEW_AUTH_LOG"
write_response() { write_response() {
local status="$1" body="$2" local status="$1" body="$2"
[[ -n "$output_file" ]] || exit 96 [[ -n "$output_file" ]] || exit 96
@@ -129,7 +174,8 @@ emit() {
mode="${PR_REVIEW_TEST_MODE:-}" mode="${PR_REVIEW_TEST_MODE:-}"
if [[ "$method" == "GET" && "$path" == "$PR_REVIEW_API_ROOT/user" ]]; then 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 json
import os import os
print(json.dumps({"login": os.environ["PR_REVIEW_LOGIN"]})) 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 elif [[ "$method" == "POST" && "$path" == "$PR_REVIEW_EXPECTED_API_BASE/pulls/123/reviews" ]]; then
printf '%s' "$payload" > "$PR_REVIEW_SUBMIT_PAYLOAD" 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 json
import os import os
@@ -321,12 +367,14 @@ run_review() {
local configured_url="${4:-https://git.mosaicstack.dev}" local configured_url="${4:-https://git.mosaicstack.dev}"
local remote_url="${5:-https://git.mosaicstack.dev/mosaicstack/stack.git}" local remote_url="${5:-https://git.mosaicstack.dev/mosaicstack/stack.git}"
local expected_repo="${6:-mosaicstack/stack}" local expected_repo="${6:-mosaicstack/stack}"
local login_override="${7:-}"
local expected_api_base="${configured_url%/}/api/v1/repos/$expected_repo" local expected_api_base="${configured_url%/}/api/v1/repos/$expected_repo"
local expected_api_root="${configured_url%/}/api/v1" local expected_api_root="${configured_url%/}/api/v1"
git -C "$REPO_DIR" remote set-url origin "$remote_url" git -C "$REPO_DIR" remote set-url origin "$remote_url"
write_credentials "$configured_url" write_credentials "$configured_url"
: > "$TEA_LOG" : > "$TEA_LOG"
: > "$CURL_LOG" : > "$CURL_LOG"
: > "$AUTH_LOG"
: > "$OUTPUT_FILE" : > "$OUTPUT_FILE"
seed_state "$mode" seed_state "$mode"
( (
@@ -337,6 +385,7 @@ run_review() {
PR_REVIEW_TEA_LOG="$TEA_LOG" \ PR_REVIEW_TEA_LOG="$TEA_LOG" \
PR_REVIEW_LOGIN_URL="${configured_url%/}" \ PR_REVIEW_LOGIN_URL="${configured_url%/}" \
PR_REVIEW_CURL_LOG="$CURL_LOG" \ PR_REVIEW_CURL_LOG="$CURL_LOG" \
PR_REVIEW_AUTH_LOG="$AUTH_LOG" \
PR_REVIEW_REVIEWS="$REVIEWS_FILE" \ PR_REVIEW_REVIEWS="$REVIEWS_FILE" \
PR_REVIEW_COMMENTS="$COMMENTS_FILE" \ PR_REVIEW_COMMENTS="$COMMENTS_FILE" \
PR_REVIEW_SUBMIT_PAYLOAD="$SUBMIT_PAYLOAD_FILE" \ PR_REVIEW_SUBMIT_PAYLOAD="$SUBMIT_PAYLOAD_FILE" \
@@ -347,7 +396,10 @@ run_review() {
PR_REVIEW_HEAD_SHA="$HEAD_SHA" \ PR_REVIEW_HEAD_SHA="$HEAD_SHA" \
PR_REVIEW_ACTING_LOGIN="$ACTING_LOGIN" \ PR_REVIEW_ACTING_LOGIN="$ACTING_LOGIN" \
PR_REVIEW_FOREIGN_LOGIN="$FOREIGN_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 ) > "$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 '^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/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" 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 assert_no_tea_write
# The submitted review payload carries the event and the PR head commit_id. # The submitted review payload carries the event and the PR head commit_id.
PR_REVIEW_HEAD_SHA="$HEAD_SHA" python3 - "$SUBMIT_PAYLOAD_FILE" <<'PY' 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 exit 1
fi 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" echo "pr-review.sh REST review + comment create/read-back regression passed"