fix(tools): write Gitea reviews/comments via REST POST and verify by exact created id (#865)
All checks were successful
ci/woodpecker/pr/ci Pipeline was successful

Replace the tea-based write + boundary/author read-back with a direct Gitea
REST POST that returns the created record's id, and verify that exact record.

BLOCKER 2 (credential ordering): resolve the acting identity, the write token,
and the read-back token from the SAME effective login. A --login override now
selects the credential used for the POST, GET /user, and the GET-by-id
read-back, so an overridden write is verified against the identity that
performed it -- not the host default. Login-name resolution is best-effort and
non-fatal (the override always wins; otherwise fall back to the host
credential), so exotic/ported hosts still resolve a token.

BLOCKER 1+3 (attribution + tautological tests): the write is now
POST /issues/{n}/comments or POST /pulls/{n}/reviews (event + body + commit_id
== PR head), parsing the provider-returned created id. Verification GETs that
exact id and checks author == acting identity and body (comments) or state +
commit_id (reviews). Keying on the created id closes the concurrency window:
a no-op create yields no id and fails closed with no list-scan fallback, and a
concurrent same-identity record has a different id. The review body travels in
the review submit, removing the separate detached comment.

Tests: the curl stub now models a real server with persistent on-disk
review/comment state -- a POST actually creates+persists a record and returns
its id, and the read-back reads that same state (no fabricated record for the
wrapper to find). Adds same-identity no-op-concurrent and author-mismatch
fail-closed cases for both comments and reviews, and >page-1 pagination
coverage for both. README "Durable review provenance" refreshed for the REST
mechanism.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
This commit is contained in:
Hermes Agent
2026-07-21 19:46:32 -05:00
parent 16481ece3d
commit 9384f0bc0a
6 changed files with 1024 additions and 687 deletions

View File

@@ -6,21 +6,24 @@ These scripts provide host-aware GitHub and Gitea issue, pull-request, milestone
A successful provider write command—or a wrapper message based only on that command's exit code—is **not** durable review provenance. Review comments, approvals, and change requests count as durable provenance only after the wrapper reads the created provider record back and verifies that it was created by _this_ write. A successful provider write command—or a wrapper message based only on that command's exit code—is **not** durable review provenance. Review comments, approvals, and change requests count as durable provenance only after the wrapper reads the created provider record back and verifies that it was created by _this_ write.
`pr-review.sh` therefore fails closed when a Gitea comment cannot be written, its created comment ID cannot be identified, or provider read-back does not match. It reports comment success only after that read-back verification passes. The `approve` and `request-changes` actions apply the same discipline to the review **state** itself: they record the maximum existing review id _before_ invoking `tea pr approve`/`reject`, then require a review whose id is strictly greater than that boundary, whose **author login equals the acting identity** (resolved via `GET /api/v1/user` for the token in use), whose state matches the requested action (`APPROVED` / `REQUEST_CHANGES`), and whose reviewed commit equals the PR's current head. tea's exit code alone is never treated as evidence the review landed. **The write is a direct Gitea REST `POST` that returns the created record's id.** Neither wrapper writes through `tea` — tea 0.11.1 can silently no-op while exiting 0 and cannot emit the id of a record it creates, so its exit code is worthless as proof of a durable write (#865). Instead:
`issue-comment.sh` applies the same fail-closed, boundary-bounded read-back to issue comments: it records the maximum existing comment id _before_ posting via `tea comment`, then re-fetches the issue's comments via the Gitea REST API and requires a comment whose id is strictly greater than that boundary, **whose author login equals the acting identity**, **and** whose body exactly matches what was submitted. Bounding the read-back by the pre-write id is essential — a body-only match across all history would falsely report success if `tea comment` silently no-ops (the #865 bug) while an identically-bodied comment already existed from a prior run. Gitea comment and review ids are monotonic, so `id > boundary` reliably means "created after this write began". - Comments (`issue-comment.sh`, and the `comment` action of `pr-review.sh`) `POST /api/v1/repos/{owner}/{repo}/issues/{index}/comments`, requiring a `201` and parsing the created comment's `id` from the response body.
- Reviews (`approve` / `request-changes`) `POST /api/v1/repos/{owner}/{repo}/pulls/{index}/reviews` with the `event` (`APPROVED` / `REQUEST_CHANGES`), the review `body`, and `commit_id` pinned to the PR's current head, then parse the created review's `id`. The review body travels _in the review submit itself_ — there is no separate detached comment to reconcile (a Gitea `REQUEST_CHANGES` review requires a non-empty body, which the submit carries).
**Invocation attribution, not just temporal ordering.** `id > boundary` alone only proves a record was created after the write began; it would still be satisfied by a _concurrent_ write from a _different_ identity while this `tea` invocation created nothing. Both wrappers therefore additionally require the accepted record's author login to equal the identity the API token authenticates as, narrowing the match to this invocation's writer. The one residual window — a concurrent write by the _same_ identity with an identical body/state inside the boundary window — cannot be eliminated without a tea-emitted created-record id, which tea 0.11.1 does not reliably provide; it is strictly narrower than temporal-only matching and is documented in-code. **Verification keys on that exact provider-returned id.** The wrapper then `GET`s that one record directly — `GET /issues/comments/{id}` or `GET /pulls/{n}/reviews/{id}` — and requires that its `id` equals the created id, its **author login equals the acting identity** (resolved via `GET /api/v1/user` for the token in use), and, for comments, its body exactly matches what was submitted, or, for reviews, its state matches the requested action and its reviewed `commit_id` equals the PR head. The write, the `/user` identity lookup, and the read-back all use the **same** credential — the effective login's token, or the host credential when no login is named — so the write is verified against the identity that actually performed it.
**Full pagination.** Gitea paginates list endpoints, so a single-page read of the comments or reviews list would false-negative once the freshly created record lands beyond the first page. Both the pre-write boundary computation and the post-write read-back walk every page (`?limit=&page=1,2,…` until a short/empty page) so the match is exhaustive regardless of how many comments or reviews already exist. **This closes the concurrency window rather than documenting it.** Because verification keys on the id the create returned, a no-op create yields no id and fails closed with no list-scan fallback, and a _concurrent_ record — even one written by the _same_ identity with an identical body/state — has a _different_ id and cannot be mistaken for this write. There is no residual same-identity window: the earlier boundary-and-author heuristic (accept any `id > pre-write-max` with a matching author) is replaced entirely by exact-id attribution.
**Full pagination.** After the exact-id read-back, each wrapper also confirms the created id is enumerable in the record list, walking every page (`?limit=&page=1,2,…` until a short/empty page) so a record that lands beyond the first page is still found regardless of how many comments or reviews already exist.
## `tea` invocation notes (Gitea) ## `tea` invocation notes (Gitea)
- tea v0.11.1 has **no `comment` subcommand under `tea pr` or `tea issue`**. The correct invocation is the **top-level** `tea comment <index> <body> [--repo ...] [--login ...]`. The `tea pr comment` / `tea issue comment` forms don't error — tea silently falls through to a no-op and still exits 0, producing a false-success write (#865). Always use the top-level form. - tea v0.11.1 has **no `comment` subcommand under `tea pr` or `tea issue`** — the `tea pr comment` / `tea issue comment` forms don't error, they silently fall through to a no-op and still exit 0, producing a false-success write (#865). tea's write subcommands (`tea comment`, `tea pr approve`/`reject`) also cannot report the id of the record they create, so their exit code cannot prove a durable write. These wrappers therefore do **not** write reviews or comments through `tea` at all; they use direct Gitea REST `POST`s that return the created record's id (see "Durable review provenance" above). `tea` is consulted only to enumerate the login list for host→login resolution.
- `tea pr approve` and `tea pr reject` take an optional review comment/reason as a **trailing positional argument**, not a `--comment`/`-comment` flag (that flag does not exist on those subcommands). `pr-review.sh` avoids this positional form entirely for the approve/reject actions and instead posts any review comment through the same durable, read-back-verified comment API used for the `comment` action (see #835/#812) — the trailing-positional form remains available to callers who invoke `tea` directly, but is not used by these wrappers. - Because the review body is carried in the `POST …/reviews` submit itself, there is no separate detached review comment, and the historical `tea pr approve`/`reject` trailing-positional-argument vs. nonexistent `--comment`/`-comment` flag hazard (#835) no longer applies to these wrappers — no review comment is ever passed to `tea`.
### `--login` passthrough ### `--login` override
Both `pr-review.sh` and `issue-comment.sh` accept an optional `--login <name>` flag that overrides the automatically detected Gitea `tea` login for that single invocation. The override is appended to the `tea` command line **after** the detected default (`get_gitea_repo_args()` / `get_gitea_login[_for_host]()`), because tea honors only the **last** `--login` flag on its command line — an override placed before the default would be silently clobbered by it. Callers who need a different login than the host default should pass `--login <reviewer-login>` rather than relying on ordering tricks or re-invoking `tea login` globally. Both `pr-review.sh` and `issue-comment.sh` accept an optional `--login <name>` flag that overrides the automatically detected Gitea login for that single invocation. The override selects **which credential the REST write, the `/user` identity lookup, and the read-back all use** — its token is resolved from the tea config for that login name (`get_gitea_token_for_login`), falling back to the repo host's credential when no login is named. Resolving the acting identity and the read-back from the _same_ login that performs the write is essential: a write performed under an overridden login must be verified against that login's identity, not the host default's. Callers who need a different login than the host default should pass `--login <reviewer-login>`.
As a durable successor to this mechanism, consider giving each reviewer/approver slot its own dedicated Gitea login credential, so that author≠reviewer holds at the credential level rather than relying on wrapper-level `--login` bookkeeping. This is a recommendation for future hardening, not something implemented by this flag. As a durable successor to this mechanism, consider giving each reviewer/approver slot its own dedicated Gitea login credential, so that author≠reviewer holds at the credential level rather than relying on wrapper-level `--login` bookkeeping. This is a recommendation for future hardening, not something implemented by this flag.

View File

@@ -563,6 +563,54 @@ get_gitea_token() {
return 1 return 1
} }
# Resolve the API token for a SPECIFIC tea login name from tea's own config
# (the same store tea itself writes/reads for `--login <name>`). This is what
# lets a REST write be performed AS the selected --login identity: tea keys its
# per-login tokens by `name` in $XDG_CONFIG_HOME/tea/config.yml (default
# ~/.config/tea/config.yml), exactly as the `tea` CLI resolves them, so a
# --login override and its REST read-back bind to the SAME credential/identity.
# Prints the token on success; returns non-zero (no output) if the config or a
# matching login token cannot be found. Callers must not log the result.
get_gitea_token_for_login() {
local login_name="$1" config_file
[[ -n "$login_name" ]] || return 1
config_file="${XDG_CONFIG_HOME:-$HOME/.config}/tea/config.yml"
[[ -f "$config_file" ]] || return 1
LOGIN_NAME="$login_name" python3 - "$config_file" <<'PY'
import os
import sys
try:
import yaml
except ImportError:
raise SystemExit(1)
try:
with open(sys.argv[1], encoding="utf-8") as handle:
config = yaml.safe_load(handle)
except (OSError, yaml.YAMLError):
raise SystemExit(1)
wanted = os.environ["LOGIN_NAME"]
logins = config.get("logins") if isinstance(config, dict) else None
if not isinstance(logins, list):
raise SystemExit(1)
for login in logins:
if not isinstance(login, dict):
continue
if str(login.get("name") or "") == wanted:
token = login.get("token")
if isinstance(token, str) and token:
print(token)
raise SystemExit(0)
break
raise SystemExit(1)
PY
}
# Resolve HTTPS basic auth credentials for a Gitea host from ~/.git-credentials. # Resolve HTTPS basic auth credentials for a Gitea host from ~/.git-credentials.
# Prints "username:password" for direct curl -u consumption. Callers must not log it. # Prints "username:password" for direct curl -u consumption. Callers must not log it.
get_gitea_basic_auth() { get_gitea_basic_auth() {

View File

@@ -3,21 +3,24 @@
# Usage: issue-comment.sh -i <issue_number> -c <comment> [--login <name>] # Usage: issue-comment.sh -i <issue_number> -c <comment> [--login <name>]
# #
# tea v0.11.1 defines no `comment` subcommand under `tea issue` (or `tea pr`); # tea v0.11.1 defines no `comment` subcommand under `tea issue` (or `tea pr`);
# the correct invocation is the TOP-LEVEL `tea comment <index> <body>` form. # the non-existent `tea issue comment ...` form does not error — tea silently
# Calling the non-existent `tea issue comment ...` form does not error — tea # no-ops and still exits 0, so a caller trusting the exit code believes a
# silently falls through to a no-op and still exits 0, so a caller trusting # comment was posted when it was not (#865). tea 0.11.1 also cannot reliably
# the exit code alone believes a comment was posted when it was not (#865). # emit the id of a record it created, so an exit code is the ONLY signal it
# Because that failure mode is silent, this script never trusts tea's exit # offers — and that signal is untrustworthy. This script therefore does not
# code alone: after posting, it independently re-fetches the issue's comments # write via tea at all: it POSTs the comment through the Gitea REST API (which
# via the Gitea REST API (curl — urllib is blocked by Cloudflare on this # returns the created comment object, including its id), then GETs that exact
# host) and fails closed if the posted body cannot be found. # id back and fails closed unless it matches. Keying verification to the
# provider-returned created id means a concurrent comment cannot masquerade as
# this write and a no-op create simply yields no id to verify.
# #
# --login override: the default `--login` is resolved from the local `tea` # --login override: the default login is resolved from the local `tea` login
# login list for this repo's host (get_gitea_login). Pass --login <name> to # list for this repo's host (get_gitea_login). Pass --login <name> to override
# override that default for this invocation only. The override is appended # it for this invocation only. The REST write, the /user identity read, and the
# to the tea command line AFTER the detected default, because tea honors # read-back are ALL performed with the token of the EFFECTIVE login (the
# only the LAST `--login` flag on the command line — a flag placed before # override when given), so the write and its verification bind to the same
# the default would be silently clobbered by it. # identity — a --login override is never written under one credential and
# verified under a different default one.
set -e set -e
@@ -72,16 +75,25 @@ fi
detect_platform >/dev/null detect_platform >/dev/null
# Resolve and cache the Gitea REST endpoint + token for the current remote. # Resolve and cache the Gitea REST endpoint + token for the current remote,
# Populates GITEA_API_ROOT (…/api/v1), GITEA_API_BASE (…/api/v1/repos/<slug>), # bound to a SPECIFIC login identity ($1). Populates GITEA_API_ROOT (…/api/v1),
# and GITEA_API_TOKEN. Returns non-zero (with a clear stderr message) if any # GITEA_API_BASE (…/api/v1/repos/<slug>), and GITEA_API_TOKEN.
# part of the resolution fails. #
gitea_resolve_api() { # The token is resolved for the EFFECTIVE login (the --login override when
local host configured_url repo # given, otherwise the detected default) so that the single credential used for
# the write ALSO drives the /user identity read and the read-back — write token
# 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.
gitea_resolve_api_for_login() {
local effective_login="$1" host configured_url repo
host=$(get_remote_host) host=$(get_remote_host)
GITEA_API_TOKEN=$(get_gitea_token "$host") || { GITEA_API_TOKEN=$(get_gitea_token_for_login "$effective_login") \
echo "Error: Gitea token not found for comment read-back verification" >&2 || GITEA_API_TOKEN=$(get_gitea_token "$host") || {
echo "Error: Gitea token not found for login '$effective_login' (comment write/read-back)" >&2
return 1 return 1
} }
configured_url=$(get_gitea_url_for_host "$host") || { configured_url=$(get_gitea_url_for_host "$host") || {
@@ -192,54 +204,22 @@ print(login)
PY PY
} }
# Print the maximum existing comment id on an issue (0 if none). This is the # Confirm that the comment CREATED by this invocation ($2 = its provider id) is
# pre-write BOUNDARY: Gitea comment ids are monotonic, so any comment created # enumerable in the issue's full, paginated comment listing and is authored by
# by a subsequent write has an id strictly greater than this value. # the acting identity. Gitea paginates list responses, so a comment created
gitea_max_comment_id() { # beyond page 1 must still be found; walking every page also proves the created
local issue_number="$1" merged_file # id is durably indexed against THIS issue rather than merely retrievable by id.
# Returns non-zero (clear stderr) if the exact created id is not present with a
merged_file=$(mktemp "${TMPDIR:-/tmp}/mosaic-issue-comment-boundary.XXXXXX") # matching author.
trap 'rm -f "$merged_file"' RETURN gitea_confirm_comment_enumerable() {
local issue_number="$1" created_id="$2" acting_login="$3" merged_file
gitea_fetch_all "$GITEA_API_BASE/issues/$issue_number/comments" "$merged_file" || return 1
python3 - "$merged_file" <<'PY'
import json
import sys
try:
with open(sys.argv[1], encoding="utf-8") as response:
comments = json.load(response)
ids = [c.get("id") for c in comments if isinstance(c, dict) and isinstance(c.get("id"), int)]
print(max(ids) if ids else 0)
except (OSError, json.JSONDecodeError, TypeError, ValueError) as error:
print(f"Error: could not compute Gitea comment boundary: {error}", file=sys.stderr)
raise SystemExit(1)
PY
}
# Independently re-fetch (all pages of) the issue's comments and require a
# comment attributable to THIS invocation: id strictly greater than the
# pre-write boundary AND author login equal to the acting identity AND exact
# body match. tea's exit code is not trustworthy evidence of a durable write
# on its own (#865); id-above-boundary alone is only temporal ordering, so the
# author-login check is what excludes a concurrent write by a DIFFERENT
# identity. Prints the matched comment ID on success.
#
# Residual (documented, not eliminable without a tea-emitted created-record
# id, which tea 0.11.1 does not reliably provide): a concurrent write by the
# SAME identity with an identical body inside the boundary window could still
# be accepted. That is a strictly narrower window than temporal-only matching.
gitea_verify_comment_posted() {
local issue_number="$1" comment_body="$2" boundary="$3" acting_login="$4"
local merged_file
merged_file=$(mktemp "${TMPDIR:-/tmp}/mosaic-issue-comment-readback.XXXXXX") merged_file=$(mktemp "${TMPDIR:-/tmp}/mosaic-issue-comment-readback.XXXXXX")
trap 'rm -f "$merged_file"' RETURN trap 'rm -f "$merged_file"' RETURN
gitea_fetch_all "$GITEA_API_BASE/issues/$issue_number/comments" "$merged_file" || return 1 gitea_fetch_all "$GITEA_API_BASE/issues/$issue_number/comments" "$merged_file" || return 1
EXPECTED_COMMENT_BODY="$comment_body" BOUNDARY_COMMENT_ID="$boundary" ACTING_LOGIN="$acting_login" \ CREATED_COMMENT_ID="$created_id" ACTING_LOGIN="$acting_login" \
python3 - "$merged_file" <<'PY' python3 - "$merged_file" <<'PY'
import json import json
import os import os
@@ -250,65 +230,161 @@ try:
comments = json.load(response) comments = json.load(response)
if not isinstance(comments, list): if not isinstance(comments, list):
raise ValueError("response is not a comment list") raise ValueError("response is not a comment list")
expected_body = os.environ["EXPECTED_COMMENT_BODY"] created_id = int(os.environ["CREATED_COMMENT_ID"])
boundary = int(os.environ["BOUNDARY_COMMENT_ID"])
acting_login = os.environ["ACTING_LOGIN"] acting_login = os.environ["ACTING_LOGIN"]
# Attribution to THIS write: created-after-boundary AND authored by the match = next(
# acting identity AND exact body match. The author check excludes a (
# concurrent DIFFERENT-identity writer that id+body alone would admit. c for c in comments
matches = [ if isinstance(c, dict)
c for c in comments and c.get("id") == created_id
if isinstance(c, dict) and (c.get("user") or {}).get("login") == acting_login
and isinstance(c.get("id"), int) ),
and c.get("id") > boundary None,
and (c.get("user") or {}).get("login") == acting_login )
and c.get("body") == expected_body if match is None:
]
if not matches:
raise ValueError( raise ValueError(
"no comment attributable to this write matched " f"created comment id {created_id} is not enumerable in the issue's "
"(id > boundary, acting identity, exact body); " "paginated comment list under the acting identity"
"tea may have silently no-opped (#865)"
) )
comment_id = max(c["id"] for c in matches) except (OSError, json.JSONDecodeError, KeyError, TypeError, ValueError) as error:
print(f"Error: Gitea comment enumeration check failed: {error}", file=sys.stderr)
raise SystemExit(1)
PY
}
# Post a comment to a Gitea issue via the supported REST API and verify it
# durably against a PROVIDER-RETURNED created id — never trust an exit code
# (#865 defect class: tea's non-existent `tea issue comment` no-ops yet exits
# 0). The write is a direct POST that returns the created comment object, so we
# learn the exact id of THIS write; we then GET that exact id and require
# id == created id AND author == acting identity AND exact body AND that it
# belongs to this issue. Because verification is keyed to the id the create
# returned, a concurrent comment (even same identity, same body) CANNOT
# masquerade as this write, and a suppressed/no-op write yields no created id
# and fails closed — there is no fallback list scan that a concurrent record
# could satisfy. Prints the created comment id on success.
#
# Args: $1 = issue number, $2 = comment body, $3 = acting identity login.
gitea_create_comment_verified() {
local issue_number="$1" comment_body="$2" acting_login="$3"
local payload write_file readback_file write_status readback_status created_id
payload=$(COMMENT_BODY="$comment_body" python3 -c '
import json
import os
print(json.dumps({"body": os.environ["COMMENT_BODY"]}))
')
write_file=$(mktemp "${TMPDIR:-/tmp}/mosaic-issue-comment-write.XXXXXX")
readback_file=$(mktemp "${TMPDIR:-/tmp}/mosaic-issue-comment-getid.XXXXXX")
trap 'rm -f "$write_file" "$readback_file"' RETURN
if ! write_status=$(curl -sS -o "$write_file" -w '%{http_code}' \
-X POST \
-H "Authorization: token $GITEA_API_TOKEN" \
-H 'Content-Type: application/json' \
-d "$payload" \
"$GITEA_API_BASE/issues/$issue_number/comments"); then
echo "Error: Gitea comment write transport failed" >&2
return 1
fi
if [[ "$write_status" != "201" ]]; then
echo "Error: Gitea comment write failed with HTTP $write_status (#865: no durable comment created)" >&2
return 1
fi
created_id=$(python3 - "$write_file" <<'PY'
import json
import sys
try:
with open(sys.argv[1], encoding="utf-8") as response:
comment = json.load(response)
created_id = comment.get("id") if isinstance(comment, dict) else None
if not isinstance(created_id, int) or created_id <= 0:
raise ValueError("create response carried no positive comment id")
except (OSError, json.JSONDecodeError, ValueError) as error:
print(f"Error: could not identify created Gitea comment: {error}", file=sys.stderr)
raise SystemExit(1)
print(created_id)
PY
) || return 1
if ! readback_status=$(curl -sS -o "$readback_file" -w '%{http_code}' \
-H "Authorization: token $GITEA_API_TOKEN" \
"$GITEA_API_BASE/issues/comments/$created_id"); then
echo "Error: Gitea comment read-back transport failed" >&2
return 1
fi
if [[ "$readback_status" != "200" ]]; then
echo "Error: Gitea comment read-back failed with HTTP $readback_status" >&2
return 1
fi
EXPECTED_COMMENT_ID="$created_id" EXPECTED_COMMENT_BODY="$comment_body" \
ACTING_LOGIN="$acting_login" EXPECTED_REPO_SLUG="${GITEA_API_BASE##*/repos/}" \
EXPECTED_ISSUE_NUMBER="$issue_number" \
python3 - "$readback_file" <<'PY' || return 1
import json
import os
import sys
from urllib.parse import urlparse
try:
with open(sys.argv[1], encoding="utf-8") as response:
comment = json.load(response)
if not isinstance(comment, dict):
raise ValueError("response is not a comment object")
expected_id = int(os.environ["EXPECTED_COMMENT_ID"])
expected_body = os.environ["EXPECTED_COMMENT_BODY"]
acting_login = os.environ["ACTING_LOGIN"]
expected_suffix = (
f"/repos/{os.environ['EXPECTED_REPO_SLUG']}"
f"/issues/{os.environ['EXPECTED_ISSUE_NUMBER']}"
)
issue_path = urlparse(comment.get("issue_url", "")).path.rstrip("/")
if comment.get("id") != expected_id:
raise ValueError("read-back id does not match the created id")
if (comment.get("user") or {}).get("login") != acting_login:
raise ValueError("created comment is not authored by the acting identity")
if comment.get("body") != expected_body:
raise ValueError("created comment body does not match")
if not issue_path.endswith(expected_suffix):
raise ValueError("created comment does not belong to this issue")
except (OSError, json.JSONDecodeError, KeyError, TypeError, ValueError) as error: except (OSError, json.JSONDecodeError, KeyError, TypeError, ValueError) as error:
print(f"Error: Gitea comment persistence verification failed: {error}", file=sys.stderr) print(f"Error: Gitea comment persistence verification failed: {error}", file=sys.stderr)
raise SystemExit(1) raise SystemExit(1)
print(comment_id)
PY PY
gitea_confirm_comment_enumerable "$issue_number" "$created_id" "$acting_login" || return 1
echo "$created_id"
return 0
} }
if [[ "$PLATFORM" == "github" ]]; then if [[ "$PLATFORM" == "github" ]]; then
gh issue comment "$ISSUE_NUMBER" --body "$COMMENT" gh issue comment "$ISSUE_NUMBER" --body "$COMMENT"
echo "Added comment to GitHub issue #$ISSUE_NUMBER" echo "Added comment to GitHub issue #$ISSUE_NUMBER"
elif [[ "$PLATFORM" == "gitea" ]]; then elif [[ "$PLATFORM" == "gitea" ]]; then
# Build the invocation as an argv array (not unquoted $(get_gitea_repo_args) # Resolve the login this comment should be attributed to: the --login
# word-splitting) so the comment body — including Markdown backticks, $(...), # override when given, otherwise the detected default for this repo's host.
# and quotes — is passed verbatim and never re-split or shell-evaluated. # A --login override always wins. Otherwise name this repo host's login only
REPO_SLUG=$(get_repo_slug) # as a best effort: the login name merely selects a per-login token, and
GITEA_LOGIN_NAME=$(get_gitea_login) || { # gitea_resolve_api_for_login falls back to the host credential
echo "Error: could not resolve a Gitea login for this repo; cannot comment on issue #$ISSUE_NUMBER." >&2 # (get_gitea_token) when no tea login is named, so the default credential
exit 1 # still resolves even when the host tea has no matching login entry.
} EFFECTIVE_LOGIN="$LOGIN_OVERRIDE"
[[ -n "$EFFECTIVE_LOGIN" ]] || EFFECTIVE_LOGIN=$(get_gitea_login 2>/dev/null || true)
# Resolve the REST endpoint, the acting identity, and the pre-write # Bind the REST endpoint + token to the effective login, then derive the
# boundary BEFORE the write, so the read-back can require a strictly-newer # acting identity from that SAME credential (GET /user). The write below and
# comment id authored by this identity. # its read-back both use this credential, so the write is verified against
gitea_resolve_api || exit 1 # the identity that actually performed it.
gitea_resolve_api_for_login "$EFFECTIVE_LOGIN" || exit 1
ACTING_LOGIN=$(gitea_authenticated_login) || exit 1 ACTING_LOGIN=$(gitea_authenticated_login) || exit 1
boundary=$(gitea_max_comment_id "$ISSUE_NUMBER") || exit 1
TEA_ARGS=(comment "$ISSUE_NUMBER" "$COMMENT" --repo "$REPO_SLUG" --login "$GITEA_LOGIN_NAME") comment_id=$(gitea_create_comment_verified "$ISSUE_NUMBER" "$COMMENT" "$ACTING_LOGIN") || {
# --login override goes LAST: tea honors only the final --login on its echo "Error: could not create and verify a comment on Gitea issue #$ISSUE_NUMBER via a provider-returned created id (#865)." >&2
# command line, so an override placed before the detected default above
# would be silently clobbered by it.
if [[ -n "$LOGIN_OVERRIDE" ]]; then
TEA_ARGS+=(--login "$LOGIN_OVERRIDE")
fi
tea "${TEA_ARGS[@]}"
comment_id=$(gitea_verify_comment_posted "$ISSUE_NUMBER" "$COMMENT" "$boundary" "$ACTING_LOGIN") || {
echo "Error: could not verify comment landed on Gitea issue #$ISSUE_NUMBER via bounded read-back; treating tea's exit code as untrustworthy (#865)." >&2
exit 1 exit 1
} }
echo "Added and verified comment on Gitea issue #$ISSUE_NUMBER (comment ID $comment_id)" echo "Added and verified comment on Gitea issue #$ISSUE_NUMBER (comment ID $comment_id)"

View File

@@ -2,16 +2,20 @@
# pr-review.sh - Review a pull request on GitHub or Gitea # pr-review.sh - Review a pull request on GitHub or Gitea
# Usage: pr-review.sh -n <pr_number> -a <action> [-c <comment>] [--login <name>] # Usage: pr-review.sh -n <pr_number> -a <action> [-c <comment>] [--login <name>]
# #
# --login override: approve/request-changes on Gitea invoke `tea pr # Gitea reviews and comments are written through the supported REST API, not
# approve`/`tea pr reject` with a `--login` resolved from the local tea # `tea`: tea 0.11.1 cannot emit the id of a record it creates and can silently
# login list for this repo's host (get_gitea_login_for_host). Pass # no-op while exiting 0 (#865 defect class), so an exit code is the only — and
# --login <name> to override that default for this invocation only. The # untrustworthy — signal it offers. approve/request-changes POST to
# override is appended to the tea command line AFTER the detected default # /pulls/{n}/reviews (returns the created review with its id); the `comment`
# (get_gitea_repo_args()-equivalent resolution happens first), because tea # action POSTs to /issues/{n}/comments (returns the created comment with its
# honors only the LAST `--login` flag on its command line — a flag placed # id). Each write is then verified by GETting that exact returned id, so a
# before the default would be silently clobbered by it. The `comment` # concurrent record cannot masquerade as this write and a no-op fails closed.
# action does not shell out to `tea` at all (see gitea_post_verified_comment #
# below), so --login has no effect on it. # --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 <name> 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.
set -e set -e
@@ -73,51 +77,37 @@ fi
detect_platform >/dev/null detect_platform >/dev/null
# Post a review comment body to a Gitea PR via the supported comments REST API # Post a comment to a Gitea PR (PR comments ARE issue comments) via the
# and verify it durably via provider read-back (see docs on durable review # supported REST API and verify it against a PROVIDER-RETURNED created id. The
# provenance in README.md). Used by the `comment` action and, since `tea` # write is a direct POST that returns the created comment object, so we learn
# v0.11.1 defines no `--comment`/`-comment` flag on `pr approve`/`pr reject`, # the exact id of THIS write; we GET that exact id and require id == created id
# also by the `approve` and `request-changes` actions to carry an optional # AND author == acting identity AND exact body AND that it belongs to this PR.
# review body that `tea` itself cannot attach. # Keying to the returned id means no concurrent comment (even same identity /
# body) can masquerade as this write, and a no-op create yields no id and fails
# closed. Requires GITEA_API_BASE / GITEA_API_TOKEN to be resolved first (via
# gitea_resolve_api_for_login). Prints the created comment id on success.
# #
# Args: $1 = PR number, $2 = comment body # Args: $1 = PR number, $2 = comment body, $3 = acting identity login.
# On success: prints only the created comment ID to stdout, returns 0. gitea_create_comment_verified() {
# On failure: prints an error to stderr, returns 1. local pr_number="$1" comment_body="$2" acting_login="$3"
gitea_post_verified_comment() { local payload write_file readback_file write_status readback_status created_id
local pr_number="$1" comment_body="$2"
local host token configured_url repo api_base payload
local write_response_file readback_response_file comment_id
host=$(get_remote_host)
token=$(get_gitea_token "$host") || {
echo "Error: Gitea token not found for comment persistence" >&2
return 1
}
configured_url=$(get_gitea_url_for_host "$host") || {
echo "Error: Configured Gitea URL not found for comment persistence" >&2
return 1
}
repo=$(get_gitea_repo_slug_for_url "$configured_url") || {
echo "Error: Could not resolve Gitea owner/repository relative to configured URL" >&2
return 1
}
api_base="${configured_url%/}/api/v1/repos/$repo"
payload=$(COMMENT_BODY="$comment_body" python3 -c ' payload=$(COMMENT_BODY="$comment_body" python3 -c '
import json import json
import os import os
print(json.dumps({"body": os.environ["COMMENT_BODY"]})) print(json.dumps({"body": os.environ["COMMENT_BODY"]}))
') ')
write_response_file=$(mktemp "${TMPDIR:-/tmp}/mosaic-pr-review-write.XXXXXX") write_file=$(mktemp "${TMPDIR:-/tmp}/mosaic-pr-review-write.XXXXXX")
readback_response_file=$(mktemp "${TMPDIR:-/tmp}/mosaic-pr-review-readback.XXXXXX") readback_file=$(mktemp "${TMPDIR:-/tmp}/mosaic-pr-review-getid.XXXXXX")
trap 'rm -f "$write_response_file" "$readback_response_file"' RETURN trap 'rm -f "$write_file" "$readback_file"' RETURN
if ! write_status=$(curl -sS -o "$write_response_file" -w '%{http_code}' \ if ! write_status=$(curl -sS -o "$write_file" -w '%{http_code}' \
-X POST \ -X POST \
-H "Authorization: token $token" \ -H "Authorization: token $GITEA_API_TOKEN" \
-H 'Content-Type: application/json' \ -H 'Content-Type: application/json' \
-d "$payload" \ -d "$payload" \
"$api_base/issues/$pr_number/comments"); then "$GITEA_API_BASE/issues/$pr_number/comments"); then
echo "Error: Gitea comment write transport failed" >&2 echo "Error: Gitea comment write transport failed" >&2
return 1 return 1
fi fi
@@ -126,26 +116,26 @@ print(json.dumps({"body": os.environ["COMMENT_BODY"]}))
return 1 return 1
fi fi
comment_id=$(python3 - "$write_response_file" <<'PY' created_id=$(python3 - "$write_file" <<'PY'
import json import json
import sys import sys
try: try:
with open(sys.argv[1], encoding="utf-8") as response: with open(sys.argv[1], encoding="utf-8") as response:
comment = json.load(response) comment = json.load(response)
comment_id = comment.get("id") if isinstance(comment, dict) else None created_id = comment.get("id") if isinstance(comment, dict) else None
if not isinstance(comment_id, int) or comment_id <= 0: if not isinstance(created_id, int) or created_id <= 0:
raise ValueError("missing positive comment id") raise ValueError("create response carried no positive comment id")
except (OSError, json.JSONDecodeError, ValueError) as error: except (OSError, json.JSONDecodeError, ValueError) as error:
print(f"Error: could not identify created Gitea comment: {error}", file=sys.stderr) print(f"Error: could not identify created Gitea comment: {error}", file=sys.stderr)
raise SystemExit(1) raise SystemExit(1)
print(comment_id) print(created_id)
PY PY
) || return 1 ) || return 1
if ! readback_status=$(curl -sS -o "$readback_response_file" -w '%{http_code}' \ if ! readback_status=$(curl -sS -o "$readback_file" -w '%{http_code}' \
-H "Authorization: token $token" \ -H "Authorization: token $GITEA_API_TOKEN" \
"$api_base/issues/comments/$comment_id"); then "$GITEA_API_BASE/issues/comments/$created_id"); then
echo "Error: Gitea comment read-back transport failed" >&2 echo "Error: Gitea comment read-back transport failed" >&2
return 1 return 1
fi fi
@@ -154,8 +144,10 @@ PY
return 1 return 1
fi fi
if EXPECTED_COMMENT_ID="$comment_id" EXPECTED_COMMENT_BODY="$comment_body" EXPECTED_REPO="$repo" EXPECTED_PR_NUMBER="$pr_number" \ EXPECTED_COMMENT_ID="$created_id" EXPECTED_COMMENT_BODY="$comment_body" \
python3 - "$readback_response_file" <<'PY' ACTING_LOGIN="$acting_login" EXPECTED_REPO_SLUG="${GITEA_API_BASE##*/repos/}" \
EXPECTED_PR_NUMBER="$pr_number" \
python3 - "$readback_file" <<'PY' || return 1
import json import json
import os import os
import sys import sys
@@ -168,40 +160,48 @@ try:
raise ValueError("response is not a comment object") raise ValueError("response is not a comment object")
expected_id = int(os.environ["EXPECTED_COMMENT_ID"]) expected_id = int(os.environ["EXPECTED_COMMENT_ID"])
expected_body = os.environ["EXPECTED_COMMENT_BODY"] expected_body = os.environ["EXPECTED_COMMENT_BODY"]
expected_repo = os.environ["EXPECTED_REPO"] acting_login = os.environ["ACTING_LOGIN"]
expected_pr = os.environ["EXPECTED_PR_NUMBER"] expected_suffix = (
f"/repos/{os.environ['EXPECTED_REPO_SLUG']}"
f"/issues/{os.environ['EXPECTED_PR_NUMBER']}"
)
issue_path = urlparse(comment.get("issue_url", "")).path.rstrip("/") issue_path = urlparse(comment.get("issue_url", "")).path.rstrip("/")
expected_suffix = f"/repos/{expected_repo}/issues/{expected_pr}"
if comment.get("id") != expected_id: if comment.get("id") != expected_id:
raise ValueError("comment id mismatch") raise ValueError("read-back id does not match the created id")
if (comment.get("user") or {}).get("login") != acting_login:
raise ValueError("created comment is not authored by the acting identity")
if comment.get("body") != expected_body: if comment.get("body") != expected_body:
raise ValueError("comment body mismatch") raise ValueError("created comment body does not match")
if not issue_path.endswith(expected_suffix): if not issue_path.endswith(expected_suffix):
raise ValueError("repository or PR mismatch") raise ValueError("created comment does not belong to this PR")
except (OSError, json.JSONDecodeError, KeyError, TypeError, ValueError) as error: except (OSError, json.JSONDecodeError, KeyError, TypeError, ValueError) as error:
print(f"Error: Gitea comment persistence verification failed: {error}", file=sys.stderr) print(f"Error: Gitea comment persistence verification failed: {error}", file=sys.stderr)
raise SystemExit(1) raise SystemExit(1)
PY PY
then
true
else
return 1
fi
echo "$comment_id" echo "$created_id"
return 0 return 0
} }
# Resolve and cache the Gitea REST endpoint + token for the current remote. # Resolve and cache the Gitea REST endpoint + token for the current remote,
# Populates GITEA_API_ROOT (…/api/v1), GITEA_API_BASE (…/api/v1/repos/<slug>), # bound to a SPECIFIC login identity ($1). Populates GITEA_API_ROOT (…/api/v1),
# and GITEA_API_TOKEN. Returns non-zero (with a clear stderr message) on any # GITEA_API_BASE (…/api/v1/repos/<slug>), and GITEA_API_TOKEN.
# resolution failure. #
gitea_resolve_api() { # The token is resolved for the EFFECTIVE login (the --login override when
local host configured_url repo # given, otherwise the detected default), so the one credential used to submit
# the review/comment ALSO drives the /user identity read and every read-back —
# 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.
gitea_resolve_api_for_login() {
local effective_login="$1" host configured_url repo
host=$(get_remote_host) host=$(get_remote_host)
GITEA_API_TOKEN=$(get_gitea_token "$host") || { GITEA_API_TOKEN=$(get_gitea_token_for_login "$effective_login") \
echo "Error: Gitea token not found for review read-back verification" >&2 || GITEA_API_TOKEN=$(get_gitea_token "$host") || {
echo "Error: Gitea token not found for login '$effective_login' (review write/read-back)" >&2
return 1 return 1
} }
configured_url=$(get_gitea_url_for_host "$host") || { configured_url=$(get_gitea_url_for_host "$host") || {
@@ -311,65 +311,17 @@ print(login)
PY PY
} }
# Print the maximum existing review id on a PR (0 if none). This is the # Resolve the PR's current head commit SHA (GET /pulls/{n}). The review is
# pre-write BOUNDARY: Gitea pull-review ids are monotonic, so any review # submitted against — and later verified as pinned to — this exact commit, so a
# submitted by a subsequent `tea pr approve`/`reject` has an id strictly # stale review left over from an earlier push cannot be mistaken for this one.
# greater than this value. Paginates fully so a boundary review beyond page 1 # Prints the head SHA on success.
# is still counted. gitea_pr_head_sha() {
gitea_max_review_id() { local pr_number="$1" pr_file status
local pr_number="$1" merged_file
merged_file=$(mktemp "${TMPDIR:-/tmp}/mosaic-pr-review-boundary.XXXXXX") pr_file=$(mktemp "${TMPDIR:-/tmp}/mosaic-pr-review-head.XXXXXX")
trap 'rm -f "$merged_file"' RETURN trap 'rm -f "$pr_file"' RETURN
gitea_fetch_all "$GITEA_API_BASE/pulls/$pr_number/reviews" "$merged_file" || return 1 if ! status=$(curl -sS -o "$pr_file" -w '%{http_code}' \
python3 - "$merged_file" <<'PY'
import json
import sys
try:
with open(sys.argv[1], encoding="utf-8") as response:
reviews = json.load(response)
ids = [r.get("id") for r in reviews if isinstance(r, dict) and isinstance(r.get("id"), int)]
print(max(ids) if ids else 0)
except (OSError, json.JSONDecodeError, TypeError, ValueError) as error:
print(f"Error: could not compute Gitea review boundary: {error}", file=sys.stderr)
raise SystemExit(1)
PY
}
# Independently verify that `tea pr approve`/`reject` produced a durable review
# record — never trust tea's exit code alone (#865, same defect class). Require
# a review attributable to THIS action: its id must be strictly greater than
# the pre-write boundary, its author login must equal the acting identity, its
# state must equal the expected state (APPROVED / REQUEST_CHANGES), and it must
# have been submitted against the PR's current head commit. The reviews list is
# paginated fully so a matching review beyond page 1 is still found. Prints the
# matched review id on success; fails closed (non-zero, clear stderr) if no
# such review is found.
#
# id-above-boundary alone is only temporal ordering; the author-login check is
# what excludes a concurrent review submitted by a DIFFERENT identity.
#
# Residual (documented, not eliminable without a tea-emitted created-record id,
# which tea 0.11.1 does not reliably provide for approve/reject): a concurrent
# review by the SAME identity with the same state against the same head inside
# the boundary window could still be accepted. That is strictly narrower than
# temporal-only matching.
#
# Args: $1 = PR number, $2 = expected state (APPROVED|REQUEST_CHANGES),
# $3 = pre-write boundary review id, $4 = acting reviewer login.
gitea_verify_review_submitted() {
local pr_number="$1" expected_state="$2" boundary="$3" acting_login="$4"
local pr_response_file reviews_merged_file status head_sha review_id
pr_response_file=$(mktemp "${TMPDIR:-/tmp}/mosaic-pr-review-head.XXXXXX")
reviews_merged_file=$(mktemp "${TMPDIR:-/tmp}/mosaic-pr-review-state.XXXXXX")
trap 'rm -f "$pr_response_file" "$reviews_merged_file"' RETURN
# Resolve the PR's current head commit so the review can be pinned to it.
if ! status=$(curl -sS -o "$pr_response_file" -w '%{http_code}' \
-H "Authorization: token $GITEA_API_TOKEN" \ -H "Authorization: token $GITEA_API_TOKEN" \
"$GITEA_API_BASE/pulls/$pr_number"); then "$GITEA_API_BASE/pulls/$pr_number"); then
echo "Error: Gitea PR head read transport failed" >&2 echo "Error: Gitea PR head read transport failed" >&2
@@ -379,7 +331,7 @@ gitea_verify_review_submitted() {
echo "Error: Gitea PR head read failed with HTTP $status" >&2 echo "Error: Gitea PR head read failed with HTTP $status" >&2
return 1 return 1
fi fi
head_sha=$(python3 - "$pr_response_file" <<'PY' python3 - "$pr_file" <<'PY'
import json import json
import sys import sys
@@ -394,12 +346,25 @@ except (OSError, json.JSONDecodeError, AttributeError, TypeError, ValueError) as
raise SystemExit(1) raise SystemExit(1)
print(head_sha) print(head_sha)
PY PY
) || return 1 }
gitea_fetch_all "$GITEA_API_BASE/pulls/$pr_number/reviews" "$reviews_merged_file" || return 1 # Confirm that the review CREATED by this action ($2 = its provider id) is
# enumerable in the PR's full, paginated review listing, authored by the acting
# identity, in the expected state. Gitea paginates review lists, so a review
# created beyond page 1 must still be found; walking every page also proves the
# created id is durably indexed against THIS PR rather than merely retrievable
# by id. Returns non-zero (clear stderr) if the exact created id is absent or
# does not match author/state.
gitea_confirm_review_enumerable() {
local pr_number="$1" created_id="$2" expected_state="$3" acting_login="$4" merged_file
review_id=$(EXPECTED_STATE="$expected_state" BOUNDARY_REVIEW_ID="$boundary" EXPECTED_HEAD_SHA="$head_sha" ACTING_LOGIN="$acting_login" \ merged_file=$(mktemp "${TMPDIR:-/tmp}/mosaic-pr-review-list.XXXXXX")
python3 - "$reviews_merged_file" <<'PY' trap 'rm -f "$merged_file"' RETURN
gitea_fetch_all "$GITEA_API_BASE/pulls/$pr_number/reviews" "$merged_file" || return 1
CREATED_REVIEW_ID="$created_id" EXPECTED_STATE="$expected_state" ACTING_LOGIN="$acting_login" \
python3 - "$merged_file" <<'PY'
import json import json
import os import os
import sys import sys
@@ -409,38 +374,138 @@ try:
reviews = json.load(response) reviews = json.load(response)
if not isinstance(reviews, list): if not isinstance(reviews, list):
raise ValueError("response is not a review list") raise ValueError("response is not a review list")
created_id = int(os.environ["CREATED_REVIEW_ID"])
expected_state = os.environ["EXPECTED_STATE"] expected_state = os.environ["EXPECTED_STATE"]
boundary = int(os.environ["BOUNDARY_REVIEW_ID"])
expected_head = os.environ["EXPECTED_HEAD_SHA"]
acting_login = os.environ["ACTING_LOGIN"] acting_login = os.environ["ACTING_LOGIN"]
# Attribution to THIS action: created-after-boundary AND submitted by the match = next(
# acting reviewer identity AND expected state AND pinned to the PR's (
# current head commit. The author check excludes a concurrent r for r in reviews
# DIFFERENT-identity review that id+state+head alone would admit. if isinstance(r, dict)
matches = [ and r.get("id") == created_id
r for r in reviews and (r.get("user") or {}).get("login") == acting_login
if isinstance(r, dict) and r.get("state") == expected_state
and isinstance(r.get("id"), int) ),
and r.get("id") > boundary None,
and (r.get("user") or {}).get("login") == acting_login )
and r.get("state") == expected_state if match is None:
and r.get("commit_id") == expected_head
]
if not matches:
raise ValueError( raise ValueError(
f"no {expected_state} review attributable to this action found " f"created review id {created_id} is not enumerable in the PR's "
"(id > boundary, acting identity, expected state, current head); " "paginated review list under the acting identity/state"
"tea may have silently failed (#865 defect class)"
) )
review_id = max(r["id"] for r in matches)
except (OSError, json.JSONDecodeError, KeyError, TypeError, ValueError) as error: except (OSError, json.JSONDecodeError, KeyError, TypeError, ValueError) as error:
print(f"Error: Gitea review persistence verification failed: {error}", file=sys.stderr) print(f"Error: Gitea review enumeration check failed: {error}", file=sys.stderr)
raise SystemExit(1) raise SystemExit(1)
print(review_id) PY
}
# Submit a review to a Gitea PR via the supported REST API and verify it against
# a PROVIDER-RETURNED created id. tea 0.11.1's `pr approve`/`reject` cannot emit
# the id of the review it created and can silently no-op while exiting 0 (#865
# defect class), so this does NOT shell out to tea: it POSTs to
# /pulls/{n}/reviews with the event (APPROVED / REQUEST_CHANGES), the PR head
# commit_id, and the review body, which returns the created review object
# including its id. It then GETs that exact review id and requires
# id == created id AND author == acting identity AND state == expected AND
# commit_id == PR head. Keying to the returned id means no concurrent review
# (even same identity/state/head) can masquerade as this one, and a no-op
# submit yields no id and fails closed. Prints the created review id on success.
#
# Args: $1 = PR number, $2 = event (APPROVED|REQUEST_CHANGES),
# $3 = review body (may be empty for APPROVED), $4 = acting login,
# $5 = PR head sha.
gitea_submit_review_verified() {
local pr_number="$1" event="$2" review_body="$3" acting_login="$4" head_sha="$5"
local payload write_file readback_file write_status readback_status created_id
payload=$(REVIEW_EVENT="$event" REVIEW_BODY="$review_body" REVIEW_COMMIT="$head_sha" python3 -c '
import json
import os
print(json.dumps({
"event": os.environ["REVIEW_EVENT"],
"body": os.environ["REVIEW_BODY"],
"commit_id": os.environ["REVIEW_COMMIT"],
}))
')
write_file=$(mktemp "${TMPDIR:-/tmp}/mosaic-pr-review-submit.XXXXXX")
readback_file=$(mktemp "${TMPDIR:-/tmp}/mosaic-pr-review-getid.XXXXXX")
trap 'rm -f "$write_file" "$readback_file"' RETURN
if ! write_status=$(curl -sS -o "$write_file" -w '%{http_code}' \
-X POST \
-H "Authorization: token $GITEA_API_TOKEN" \
-H 'Content-Type: application/json' \
-d "$payload" \
"$GITEA_API_BASE/pulls/$pr_number/reviews"); then
echo "Error: Gitea review submit transport failed" >&2
return 1
fi
# Gitea returns 200 (occasionally 201) with the created review object.
if [[ "$write_status" != "200" && "$write_status" != "201" ]]; then
echo "Error: Gitea review submit failed with HTTP $write_status (#865: no durable review created)" >&2
return 1
fi
created_id=$(python3 - "$write_file" <<'PY'
import json
import sys
try:
with open(sys.argv[1], encoding="utf-8") as response:
review = json.load(response)
created_id = review.get("id") if isinstance(review, dict) else None
if not isinstance(created_id, int) or created_id <= 0:
raise ValueError("submit response carried no positive review id")
except (OSError, json.JSONDecodeError, ValueError) as error:
print(f"Error: could not identify created Gitea review: {error}", file=sys.stderr)
raise SystemExit(1)
print(created_id)
PY PY
) || return 1 ) || return 1
echo "$review_id" if ! readback_status=$(curl -sS -o "$readback_file" -w '%{http_code}' \
-H "Authorization: token $GITEA_API_TOKEN" \
"$GITEA_API_BASE/pulls/$pr_number/reviews/$created_id"); then
echo "Error: Gitea review read-back transport failed" >&2
return 1
fi
if [[ "$readback_status" != "200" ]]; then
echo "Error: Gitea review read-back failed with HTTP $readback_status" >&2
return 1
fi
EXPECTED_REVIEW_ID="$created_id" EXPECTED_STATE="$event" ACTING_LOGIN="$acting_login" \
EXPECTED_HEAD_SHA="$head_sha" \
python3 - "$readback_file" <<'PY' || return 1
import json
import os
import sys
try:
with open(sys.argv[1], encoding="utf-8") as response:
review = json.load(response)
if not isinstance(review, dict):
raise ValueError("response is not a review object")
expected_id = int(os.environ["EXPECTED_REVIEW_ID"])
expected_state = os.environ["EXPECTED_STATE"]
acting_login = os.environ["ACTING_LOGIN"]
expected_head = os.environ["EXPECTED_HEAD_SHA"]
if review.get("id") != expected_id:
raise ValueError("read-back id does not match the created id")
if (review.get("user") or {}).get("login") != acting_login:
raise ValueError("created review is not authored by the acting identity")
if review.get("state") != expected_state:
raise ValueError("created review is not in the expected state")
if review.get("commit_id") != expected_head:
raise ValueError("created review is not pinned to the PR head commit")
except (OSError, json.JSONDecodeError, KeyError, TypeError, ValueError) as error:
print(f"Error: Gitea review persistence verification failed: {error}", file=sys.stderr)
raise SystemExit(1)
PY
gitea_confirm_review_enumerable "$pr_number" "$created_id" "$event" "$acting_login" || return 1
echo "$created_id"
return 0 return 0
} }
@@ -474,74 +539,76 @@ if [[ "$PLATFORM" == "github" ]]; then
elif [[ "$PLATFORM" == "gitea" ]]; then elif [[ "$PLATFORM" == "gitea" ]]; then
case $ACTION in case $ACTION in
approve) approve)
repo=$(get_repo_slug)
host=$(get_remote_host) host=$(get_remote_host)
login=$(get_gitea_login_for_host "$host") # A --login override always wins. Otherwise name this host's login
# Resolve the REST endpoint and record the pre-write review-id # only as a best effort: the login name merely selects a per-login
# boundary BEFORE the write, so the read-back can require a # token, and gitea_resolve_api_for_login falls back to the host
# strictly-newer review created by THIS action (never trust tea's # credential (get_gitea_token) when no tea login is named — so a host
# exit code alone — #865 defect class applies to the review state). # tea's login list need not enumerate exotic (e.g. ported) hosts for
gitea_resolve_api || exit 1 # the default credential to resolve. 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.
gitea_resolve_api_for_login "$EFFECTIVE_LOGIN" || exit 1
ACTING_LOGIN=$(gitea_authenticated_login) || exit 1 ACTING_LOGIN=$(gitea_authenticated_login) || exit 1
review_boundary=$(gitea_max_review_id "$PR_NUMBER") || exit 1 head_sha=$(gitea_pr_head_sha "$PR_NUMBER") || exit 1
# tea v0.11.1 defines no --comment/-comment flag on `pr approve`; # The review body (if any) travels with the review itself in the REST
# route any review body via the durable comment API instead (#835). # submit — the created review record carries it — so there is no
TEA_ARGS=(pr approve "$PR_NUMBER" --repo "$repo" --login "$login") # separate detached comment to reconcile.
# --login override goes LAST: tea honors only the final --login on review_id=$(gitea_submit_review_verified "$PR_NUMBER" "APPROVED" "$COMMENT" "$ACTING_LOGIN" "$head_sha") || {
# its command line, so an override placed before the detected echo "Error: could not submit and verify an APPROVED review on Gitea PR #$PR_NUMBER via a provider-returned created id (#865)." >&2
# default above would be silently clobbered by it.
if [[ -n "$LOGIN_OVERRIDE" ]]; then
TEA_ARGS+=(--login "$LOGIN_OVERRIDE")
fi
tea "${TEA_ARGS[@]}"
review_id=$(gitea_verify_review_submitted "$PR_NUMBER" "APPROVED" "$review_boundary" "$ACTING_LOGIN") || {
echo "Error: could not verify an APPROVED review landed on Gitea PR #$PR_NUMBER via bounded read-back; treating tea's exit code as untrustworthy (#865)." >&2
exit 1 exit 1
} }
echo "Approved and verified Gitea PR #$PR_NUMBER (review ID $review_id)" echo "Approved and verified Gitea PR #$PR_NUMBER (review ID $review_id)"
if [[ -n "$COMMENT" ]]; then
comment_id=$(gitea_post_verified_comment "$PR_NUMBER" "$COMMENT") || exit 1
echo "Added and verified review comment on Gitea PR #$PR_NUMBER (comment ID $comment_id)"
fi
;; ;;
request-changes) request-changes)
if [[ -z "$COMMENT" ]]; then if [[ -z "$COMMENT" ]]; then
echo "Error: Comment required for request-changes" echo "Error: Comment required for request-changes"
exit 1 exit 1
fi fi
repo=$(get_repo_slug)
host=$(get_remote_host) host=$(get_remote_host)
login=$(get_gitea_login_for_host "$host") # A --login override always wins. Otherwise name this host's login
# Record the pre-write review-id boundary BEFORE the write (see the # only as a best effort: the login name merely selects a per-login
# approve path above for the rationale). # token, and gitea_resolve_api_for_login falls back to the host
gitea_resolve_api || exit 1 # 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.
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
ACTING_LOGIN=$(gitea_authenticated_login) || exit 1 ACTING_LOGIN=$(gitea_authenticated_login) || exit 1
review_boundary=$(gitea_max_review_id "$PR_NUMBER") || exit 1 head_sha=$(gitea_pr_head_sha "$PR_NUMBER") || exit 1
# tea v0.11.1 defines no --comment/-comment flag on `pr reject`; review_id=$(gitea_submit_review_verified "$PR_NUMBER" "REQUEST_CHANGES" "$COMMENT" "$ACTING_LOGIN" "$head_sha") || {
# route the review body via the durable comment API instead (#835). echo "Error: could not submit and verify a REQUEST_CHANGES review on Gitea PR #$PR_NUMBER via a provider-returned created id (#865)." >&2
TEA_ARGS=(pr reject "$PR_NUMBER" --repo "$repo" --login "$login")
# --login override goes LAST: tea honors only the final --login on
# its command line, so an override placed before the detected
# default above would be silently clobbered by it.
if [[ -n "$LOGIN_OVERRIDE" ]]; then
TEA_ARGS+=(--login "$LOGIN_OVERRIDE")
fi
tea "${TEA_ARGS[@]}"
review_id=$(gitea_verify_review_submitted "$PR_NUMBER" "REQUEST_CHANGES" "$review_boundary" "$ACTING_LOGIN") || {
echo "Error: could not verify a REQUEST_CHANGES review landed on Gitea PR #$PR_NUMBER via bounded read-back; treating tea's exit code as untrustworthy (#865)." >&2
exit 1 exit 1
} }
echo "Requested changes and verified on Gitea PR #$PR_NUMBER (review ID $review_id)" echo "Requested changes and verified on Gitea PR #$PR_NUMBER (review ID $review_id)"
comment_id=$(gitea_post_verified_comment "$PR_NUMBER" "$COMMENT") || exit 1
echo "Added and verified review comment on Gitea PR #$PR_NUMBER (comment ID $comment_id)"
;; ;;
comment) comment)
if [[ -z "$COMMENT" ]]; then if [[ -z "$COMMENT" ]]; then
echo "Error: Comment required" echo "Error: Comment required"
exit 1 exit 1
fi fi
host=$(get_remote_host)
comment_id=$(gitea_post_verified_comment "$PR_NUMBER" "$COMMENT") || exit 1 # 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.
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
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
exit 1
}
echo "Added and verified comment on Gitea PR #$PR_NUMBER (comment ID $comment_id)" echo "Added and verified comment on Gitea PR #$PR_NUMBER (comment ID $comment_id)"
;; ;;
*) *)

View File

@@ -1,28 +1,33 @@
#!/usr/bin/env bash #!/usr/bin/env bash
# Regression harness for issue-comment.sh top-level `tea comment` invocation and # Regression harness for issue-comment.sh's Gitea comment write + verification
# its BOUNDED, INVOCATION-ATTRIBUTED, PAGINATED read-back verification (#865). # (#865).
# #
# The #865 bug: `tea issue comment ...` (a nonexistent subcommand on tea # The #865 defect class: tea 0.11.1's `tea issue comment ...` (a nonexistent
# v0.11.1) silently no-ops and exits 0, so a comment is never posted. A naive # subcommand) silently no-ops yet exits 0, and tea cannot emit the id of a
# read-back that matches ANY historical comment by body would falsely report # record it created — so an exit code is worthless as proof of a durable write.
# success whenever an identically-bodied comment already exists from a prior # The wrapper therefore does NOT write via tea at all. It POSTs the comment to
# run. Merely bounding by "id > pre-write max" is also insufficient: it accepts # the Gitea REST API (which returns the created comment object, including its
# ANY newer matching comment, including one a CONCURRENT DIFFERENT identity # id), then GETs THAT EXACT id back and requires it to match on id, author
# posted while this tea invocation created nothing. This harness proves the # (acting identity), body, and issue. Because verification is keyed to the id
# wrapper: # the create returned, no concurrent comment can masquerade as this write, and a
# 1. uses the top-level `tea comment` form (never `tea issue comment`); # suppressed/no-op create yields no id and fails closed.
# 2. records the pre-write maximum comment id as a boundary and requires a
# strictly-newer comment on read-back, so a pre-existing identical body
# does NOT satisfy verification (fails closed);
# 3. attributes the matched comment to the acting identity (GET /user login),
# so a concurrent DIFFERENT-identity write does NOT satisfy verification;
# 4. reports success only when a genuinely new comment (id > boundary) with
# the exact body AND the acting author appears.
# #
# The `tea` stub NEVER creates a comment (it mimics the silent no-op); the # This harness models a REAL server: the curl stub keeps persistent comment
# "server" comment state is modeled entirely by the curl stub's responses, so # state on disk, the POST actually CREATES and PERSISTS a record and returns its
# the fresh-success vs. no-op distinction is driven purely by whether the # id, and the read-back GET reads that same state. There is no independently
# post-write read-back surfaces a new, correctly-attributed id. # fabricated record for the wrapper to "find" — the only way verification
# passes is if the POST genuinely created the record the read-back retrieves.
# It proves the wrapper:
# 1. never shells out to tea to write (no `tea comment` / `tea issue comment`);
# 2. creates the comment via REST POST and learns the provider-returned id;
# 3. verifies THAT EXACT id by direct GET, attributed to the acting identity;
# 4. fails closed when the write is a no-op even though a concurrent
# SAME-IDENTITY comment with the same body already exists (the closed
# concurrency window — no fallback list scan can rescue a no-op);
# 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.
set -euo pipefail set -euo pipefail
@@ -30,22 +35,24 @@ SCRIPT_DIR="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)"
WORK_DIR="${MOSAIC_TEST_WORK_DIR:-$PWD/.mosaic-test-work/issue-comment-readback}" WORK_DIR="${MOSAIC_TEST_WORK_DIR:-$PWD/.mosaic-test-work/issue-comment-readback}"
REPO_DIR="$WORK_DIR/repo" REPO_DIR="$WORK_DIR/repo"
BIN_DIR="$WORK_DIR/bin" BIN_DIR="$WORK_DIR/bin"
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"
OUTPUT_FILE="$WORK_DIR/output.log" OUTPUT_FILE="$WORK_DIR/output.log"
CREDENTIALS_FILE="$WORK_DIR/credentials.json" CREDENTIALS_FILE="$WORK_DIR/credentials.json"
CALLS_FILE="$WORK_DIR/comment_calls" STATE_FILE="$WORK_DIR/comments.json"
cleanup() { cleanup() {
rm -rf "$WORK_DIR" rm -rf "$WORK_DIR"
} }
trap cleanup EXIT trap cleanup EXIT
mkdir -p "$REPO_DIR" "$BIN_DIR" mkdir -p "$REPO_DIR" "$BIN_DIR" "$XDG_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
ISSUE_NUMBER=7 ISSUE_NUMBER=7
REPO_SLUG="mosaicstack/stack"
API_BASE="https://git.mosaicstack.dev/api/v1/repos/mosaicstack/stack" API_BASE="https://git.mosaicstack.dev/api/v1/repos/mosaicstack/stack"
API_ROOT="https://git.mosaicstack.dev/api/v1" API_ROOT="https://git.mosaicstack.dev/api/v1"
BODY='durable "note" -- marker' BODY='durable "note" -- marker'
@@ -68,8 +75,8 @@ with open(sys.argv[1], "w", encoding="utf-8") as credentials:
}, credentials) }, credentials)
PY PY
# tea stub: resolves the login list, and treats `tea comment ...` as a silent # tea stub: only ever answers the login list (used to resolve the default login
# no-op (exit 0 without creating anything) to mimic the real failure mode. # name). It must NEVER be asked to write a comment — the wrapper writes via REST.
cat > "$BIN_DIR/tea" <<'SH' cat > "$BIN_DIR/tea" <<'SH'
#!/usr/bin/env bash #!/usr/bin/env bash
set -euo pipefail set -euo pipefail
@@ -81,29 +88,16 @@ if [[ "$*" == "login list --output json" ]]; then
exit 0 exit 0
fi fi
# The wrapper must use the TOP-LEVEL `tea comment` form; the broken echo "Unexpected tea command (wrapper must not write via tea): $*" >&2
# `tea issue comment` subcommand must never be invoked.
if [[ "$*" == issue\ comment* ]]; then
echo "wrapper invoked nonexistent 'tea issue comment' subcommand" >&2
exit 90
fi
if [[ "$*" == comment\ * ]]; then
# Mimic tea v0.11.1: exit 0. Whether a comment actually lands is modeled
# by the curl stub's post-write read-back response, not here.
exit 0
fi
echo "Unexpected tea command: $*" >&2
exit 92 exit 92
SH SH
chmod +x "$BIN_DIR/tea" chmod +x "$BIN_DIR/tea"
# curl stub: serves GET /user (acting identity) and GET .../issues/7/comments # curl stub: a small REST server backed by persistent on-disk comment state.
# (paginated: ?limit=&page=). The comments endpoint's first call = pre-write # GET /user -> acting identity
# boundary, second call = post-write read-back. The boundary always contains a # POST /issues/7/comments -> CREATE + PERSIST, return created object
# pre-existing comment (id 50) whose body is IDENTICAL to the one under test, # GET /issues/comments/{id} -> read the persisted record by exact id
# which is exactly the condition a body-only match would trip over. # GET /issues/7/comments?page=&.. -> paginated listing of persisted state
cat > "$BIN_DIR/curl" <<'SH' cat > "$BIN_DIR/curl" <<'SH'
#!/usr/bin/env bash #!/usr/bin/env bash
set -euo pipefail set -euo pipefail
@@ -111,22 +105,22 @@ set -euo pipefail
output_file="" output_file=""
method="GET" method="GET"
url="" url=""
data=""
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 ;; -w|-H) shift 2 ;;
-X) method="$2"; shift 2 ;; -X) method="$2"; shift 2 ;;
-d|--data) shift 2 ;; -d|--data) data="$2"; shift 2 ;;
-s|-S|-sS) shift ;; -s|-S|-sS) shift ;;
http://*|https://*) url="$1"; shift ;; http://*|https://*) url="$1"; shift ;;
*) shift ;; *) shift ;;
esac esac
done done
# Strip any query string so pagination params don't defeat path matching, but
# still log the full URL (including ?limit=&page=) so the test can assert the
# read-back paginated.
path="${url%%\?*}" path="${url%%\?*}"
query="${url#*\?}"
[[ "$query" == "$url" ]] && query=""
printf '%s %s\n' "$method" "$url" >> "$ISSUE_COMMENT_CURL_LOG" printf '%s %s\n' "$method" "$url" >> "$ISSUE_COMMENT_CURL_LOG"
write_response() { write_response() {
@@ -143,55 +137,82 @@ import os
print(json.dumps({"login": os.environ["ISSUE_COMMENT_LOGIN"]})) print(json.dumps({"login": os.environ["ISSUE_COMMENT_LOGIN"]}))
PY PY
)" )"
elif [[ "$method" == "GET" && "$path" == "$ISSUE_COMMENT_API_BASE/issues/7/comments" ]]; then elif [[ "$method" == "POST" && "$path" == "$ISSUE_COMMENT_API_BASE/issues/7/comments" ]]; then
calls_file="$ISSUE_COMMENT_CALLS" result=$(ISSUE_COMMENT_DATA="$data" python3 - <<'PY'
if [[ -f "$calls_file" ]]; then
# post-write read-back
response=$(ISSUE_COMMENT_BODY="$ISSUE_COMMENT_EXPECTED_BODY" \
ISSUE_COMMENT_ACTING_LOGIN="$ISSUE_COMMENT_ACTING_LOGIN" \
ISSUE_COMMENT_FOREIGN_LOGIN="$ISSUE_COMMENT_FOREIGN_LOGIN" \
ISSUE_COMMENT_TEST_MODE="$ISSUE_COMMENT_TEST_MODE" python3 - <<'PY'
import json import json
import os import os
body = os.environ["ISSUE_COMMENT_BODY"] state_path = os.environ["ISSUE_COMMENT_STATE"]
mode = os.environ["ISSUE_COMMENT_TEST_MODE"]
acting = os.environ["ISSUE_COMMENT_ACTING_LOGIN"] acting = os.environ["ISSUE_COMMENT_ACTING_LOGIN"]
foreign = os.environ["ISSUE_COMMENT_FOREIGN_LOGIN"] foreign = os.environ["ISSUE_COMMENT_FOREIGN_LOGIN"]
mode = os.environ["ISSUE_COMMENT_TEST_MODE"] repo = os.environ["ISSUE_COMMENT_REPO_SLUG"]
body = json.loads(os.environ["ISSUE_COMMENT_DATA"]).get("body")
if mode == "fresh-success": with open(state_path, encoding="utf-8") as handle:
# A genuinely new comment (id 60 > boundary 50) authored by the acting comments = json.load(handle)
# identity.
records = [ # no-op-concurrent: the wrapper's own write is SUPPRESSED (returns 200 with no
{"id": 50, "body": body, "user": {"login": acting}}, # created object) even though a concurrent same-identity comment already exists
{"id": 60, "body": body, "user": {"login": acting}}, # in state. Nothing is persisted; there is no created id to verify.
] if mode == "no-op-concurrent":
elif mode == "foreign-identity": print("200")
# A concurrent new comment (id 60 > boundary 50) with the SAME body but a print(json.dumps({}))
# DIFFERENT author. tea created nothing; attribution must reject this. raise SystemExit(0)
records = [
{"id": 50, "body": body, "user": {"login": acting}}, author = foreign if mode == "author-mismatch" else acting
{"id": 60, "body": body, "user": {"login": foreign}}, new_id = (max((c["id"] for c in comments), default=0)) + 1
] record = {
else: "id": new_id,
# no-op: nothing new landed; the pre-existing id-50 comment remains. "body": body,
records = [{"id": 50, "body": body, "user": {"login": acting}}] "user": {"login": author},
print(json.dumps(records)) "issue_url": f"https://git.mosaicstack.dev/api/v1/repos/{repo}/issues/7",
}
comments.append(record)
with open(state_path, "w", encoding="utf-8") as handle:
json.dump(comments, handle)
print("201")
print(json.dumps(record))
PY PY
) )
else write_response "$(printf '%s' "$result" | head -n1)" "$(printf '%s' "$result" | tail -n +2)"
: > "$calls_file" elif [[ "$method" == "GET" && "$path" == "$ISSUE_COMMENT_API_BASE"/issues/comments/* ]]; then
response=$(ISSUE_COMMENT_BODY="$ISSUE_COMMENT_EXPECTED_BODY" \ result=$(ISSUE_COMMENT_GET_ID="${path##*/}" python3 - <<'PY'
ISSUE_COMMENT_ACTING_LOGIN="$ISSUE_COMMENT_ACTING_LOGIN" python3 - <<'PY'
import json import json
import os import os
body = os.environ["ISSUE_COMMENT_BODY"]
acting = os.environ["ISSUE_COMMENT_ACTING_LOGIN"] state_path = os.environ["ISSUE_COMMENT_STATE"]
print(json.dumps([{"id": 50, "body": body, "user": {"login": acting}}])) wanted = int(os.environ["ISSUE_COMMENT_GET_ID"])
with open(state_path, encoding="utf-8") as handle:
comments = json.load(handle)
match = next((c for c in comments if c["id"] == wanted), None)
if match is None:
print("404")
print(json.dumps({"message": "not found"}))
else:
print("200")
print(json.dumps(match))
PY PY
) )
fi write_response "$(printf '%s' "$result" | head -n1)" "$(printf '%s' "$result" | tail -n +2)"
write_response 200 "$response" elif [[ "$method" == "GET" && "$path" == "$ISSUE_COMMENT_API_BASE/issues/7/comments" ]]; then
result=$(ISSUE_COMMENT_QUERY="$query" python3 - <<'PY'
import json
import os
from urllib.parse import parse_qs
state_path = os.environ["ISSUE_COMMENT_STATE"]
params = parse_qs(os.environ["ISSUE_COMMENT_QUERY"])
limit = int(params.get("limit", ["50"])[0])
page = int(params.get("page", ["1"])[0])
with open(state_path, encoding="utf-8") as handle:
comments = json.load(handle)
start = (page - 1) * limit
print("200")
print(json.dumps(comments[start:start + limit]))
PY
)
write_response "$(printf '%s' "$result" | head -n1)" "$(printf '%s' "$result" | tail -n +2)"
else else
echo "Unexpected curl request: $method $url" >&2 echo "Unexpected curl request: $method $url" >&2
exit 97 exit 97
@@ -199,67 +220,112 @@ fi
SH SH
chmod +x "$BIN_DIR/curl" chmod +x "$BIN_DIR/curl"
# Seed persistent server state for a mode, then run the wrapper against it.
seed_state() {
local mode="$1"
ISSUE_COMMENT_SEED_MODE="$mode" ISSUE_COMMENT_SEED_BODY="$BODY" \
ISSUE_COMMENT_SEED_ACTING="$ACTING_LOGIN" ISSUE_COMMENT_SEED_REPO="$REPO_SLUG" \
python3 - "$STATE_FILE" <<'PY'
import json
import os
import sys
mode = os.environ["ISSUE_COMMENT_SEED_MODE"]
body = os.environ["ISSUE_COMMENT_SEED_BODY"]
acting = os.environ["ISSUE_COMMENT_SEED_ACTING"]
repo = os.environ["ISSUE_COMMENT_SEED_REPO"]
issue_url = f"https://git.mosaicstack.dev/api/v1/repos/{repo}/issues/7"
if mode == "fresh-success":
# 50 pre-existing comments fill page 1 (limit 50); the comment this run
# creates becomes id 51 and lands ALONE on page 2, exercising >page-1
# pagination in the enumeration check.
comments = [
{"id": i, "body": f"prior {i}", "user": {"login": acting}, "issue_url": issue_url}
for i in range(1, 51)
]
elif mode == "no-op-concurrent":
# A concurrent SAME-IDENTITY comment with the IDENTICAL body already exists.
# The wrapper's own write will be a no-op; it must still fail closed because
# no created id is returned — it must not scan and accept this record.
comments = [
{"id": 55, "body": body, "user": {"login": acting}, "issue_url": issue_url}
]
else: # author-mismatch
comments = []
with open(sys.argv[1], "w", encoding="utf-8") as handle:
json.dump(comments, handle)
PY
}
run_comment() { run_comment() {
local mode="$1" local mode="$1"
: > "$TEA_LOG" : > "$TEA_LOG"
: > "$CURL_LOG" : > "$CURL_LOG"
: > "$OUTPUT_FILE" : > "$OUTPUT_FILE"
rm -f "$CALLS_FILE" seed_state "$mode"
( (
cd "$REPO_DIR" cd "$REPO_DIR"
PATH="$BIN_DIR:$PATH" \ PATH="$BIN_DIR:$PATH" \
XDG_CONFIG_HOME="$XDG_DIR" \
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_CALLS="$CALLS_FILE" \ ISSUE_COMMENT_STATE="$STATE_FILE" \
ISSUE_COMMENT_TEST_MODE="$mode" \ ISSUE_COMMENT_TEST_MODE="$mode" \
ISSUE_COMMENT_EXPECTED_BODY="$BODY" \
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_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
} }
# Case 1: silent no-op with a pre-existing identical body must FAIL CLOSED. # Case 1: a genuine REST create (id 51) is verified end to end via its exact
if run_comment noop-preexisting; then # provider-returned id and enumerated on page 2 of the paginated listing.
echo "FAIL: wrapper reported success when tea no-opped but an identical body pre-existed" >&2
cat "$OUTPUT_FILE" >&2
exit 1
fi
if grep -q 'Added and verified comment' "$OUTPUT_FILE"; then
echo "FAIL: read-back matched a pre-existing comment by body only" >&2
exit 1
fi
# The wrapper must have used the top-level form and read comments back twice
# (boundary + post-write).
grep -q "^comment 7 " "$TEA_LOG"
if grep -q '^issue comment' "$TEA_LOG"; then
echo "FAIL: wrapper used the broken 'tea issue comment' subcommand" >&2
exit 1
fi
[[ "$(grep -c "^GET $API_BASE/issues/7/comments?" "$CURL_LOG")" == "2" ]]
# Read-back must be paginated (limit + page query params present).
grep -q "^GET $API_BASE/issues/7/comments?limit=[0-9]*&page=1$" "$CURL_LOG"
# Attribution must have resolved the acting identity via GET /user.
grep -q "^GET $API_ROOT/user$" "$CURL_LOG"
# Case 2: a concurrent DIFFERENT-identity write (id 60 > boundary, same body,
# foreign author) must FAIL CLOSED — temporal ordering is not attribution.
if run_comment foreign-identity; then
echo "FAIL: wrapper accepted a concurrent comment authored by a different identity" >&2
cat "$OUTPUT_FILE" >&2
exit 1
fi
if grep -q 'Added and verified comment' "$OUTPUT_FILE"; then
echo "FAIL: read-back matched a different-identity comment (attribution bypassed)" >&2
exit 1
fi
# Case 3: a genuinely new comment (id 60 > boundary 50, acting author) verifies.
run_comment fresh-success run_comment fresh-success
grep -q 'Added and verified comment on Gitea issue #7 (comment ID 60)' "$OUTPUT_FILE" grep -q 'Added and verified comment on Gitea issue #7 (comment ID 51)' "$OUTPUT_FILE"
grep -q "^comment 7 " "$TEA_LOG" # The write is a REST POST, never a tea comment.
grep -q "^POST $API_BASE/issues/7/comments$" "$CURL_LOG"
if grep -Eq '^comment |^issue comment ' "$TEA_LOG"; then
echo "FAIL: wrapper wrote a comment via tea instead of REST" >&2
exit 1
fi
# Read-back is a DIRECT GET of the exact created id.
grep -q "^GET $API_BASE/issues/comments/51$" "$CURL_LOG"
# Acting identity resolved via GET /user.
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"
echo "issue-comment.sh bounded + attributed read-back regression passed" # Case 2: a no-op write with a concurrent SAME-IDENTITY, same-body comment
# already present must FAIL CLOSED — the closed concurrency window.
if run_comment no-op-concurrent; then
echo "FAIL: wrapper reported success when its write no-opped but a concurrent same-identity comment existed" >&2
cat "$OUTPUT_FILE" >&2
exit 1
fi
if grep -q 'Added and verified comment' "$OUTPUT_FILE"; then
echo "FAIL: wrapper accepted a concurrent record for a no-op write (window not closed)" >&2
exit 1
fi
# It must NOT have fallen back to a list scan that could find the concurrent id.
if grep -q "^GET $API_BASE/issues/comments/55$" "$CURL_LOG"; then
echo "FAIL: wrapper read back the concurrent comment id 55 (illegitimate fallback)" >&2
exit 1
fi
# Case 3: a created record NOT authored by the acting identity must FAIL CLOSED.
if run_comment author-mismatch; then
echo "FAIL: wrapper accepted a created comment authored by a different identity" >&2
cat "$OUTPUT_FILE" >&2
exit 1
fi
if grep -q 'Added and verified comment' "$OUTPUT_FILE"; then
echo "FAIL: read-back did not enforce acting-identity authorship" >&2
exit 1
fi
echo "issue-comment.sh REST create + exact-id read-back regression passed"

View File

@@ -1,11 +1,23 @@
#!/usr/bin/env bash #!/usr/bin/env bash
# Regression harness for durable Gitea PR review comments (#812) and for the # Regression harness for pr-review.sh's Gitea review + comment writes (#865,
# approve/reject `--comment` flag removal (#835). The `tea` stub below rejects # #812, #835).
# any `-comment`/`--comment` flag on `pr approve`/`pr reject` exactly like real #
# `tea` v0.11.1 does ("flag provided but not defined: -comment"), so this # The #865 defect class: tea 0.11.1 can silently no-op while exiting 0 and
# harness fails RED against the pre-#835 wrapper (which passed that flag) and # cannot emit the id of a record it creates, so its exit code is worthless as
# only passes once the wrapper routes the review body through the durable # proof of a durable write. The wrapper therefore does NOT write reviews or
# comment REST API instead. # comments via tea. approve/request-changes POST to /pulls/{n}/reviews (with the
# event, the PR head commit_id, and the review body) and read the created review
# back by its EXACT provider-returned id; the `comment` action POSTs to
# /issues/{n}/comments and reads that created comment back by its exact id.
# Because verification keys on the id the create returned, no concurrent record
# can masquerade as this write and a no-op create fails closed. tea is only ever
# consulted for the login list.
#
# The curl stub models a REAL server with persistent review/comment state on
# disk: a POST actually CREATES and PERSISTS a record and returns its id, and
# 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.
set -euo pipefail set -euo pipefail
@@ -13,6 +25,11 @@ SCRIPT_DIR="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)"
WORK_DIR="${MOSAIC_TEST_WORK_DIR:-$PWD/.mosaic-test-work/pr-review-gitea-comment}" WORK_DIR="${MOSAIC_TEST_WORK_DIR:-$PWD/.mosaic-test-work/pr-review-gitea-comment}"
REPO_DIR="$WORK_DIR/repo" REPO_DIR="$WORK_DIR/repo"
BIN_DIR="$WORK_DIR/bin" BIN_DIR="$WORK_DIR/bin"
XDG_DIR="$WORK_DIR/xdg"
STATE_DIR="$WORK_DIR/state"
REVIEWS_FILE="$STATE_DIR/reviews.json"
COMMENTS_FILE="$STATE_DIR/comments.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"
OUTPUT_FILE="$WORK_DIR/output.log" OUTPUT_FILE="$WORK_DIR/output.log"
@@ -25,8 +42,9 @@ trap cleanup EXIT
ACTING_LOGIN="review-bot" ACTING_LOGIN="review-bot"
FOREIGN_LOGIN="other-writer" FOREIGN_LOGIN="other-writer"
HEAD_SHA="HEADSHA_FEEDFACE"
mkdir -p "$REPO_DIR" "$BIN_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
@@ -49,6 +67,9 @@ with open(sys.argv[1], "w", encoding="utf-8") as credentials:
PY PY
} }
# tea stub: only ever answers the login list. The wrapper must never write a
# review or comment through tea (#865 defect class); any other tea invocation is
# an error.
cat > "$BIN_DIR/tea" <<'SH' cat > "$BIN_DIR/tea" <<'SH'
#!/usr/bin/env bash #!/usr/bin/env bash
set -euo pipefail set -euo pipefail
@@ -56,42 +77,17 @@ set -euo pipefail
printf '%s\n' "$*" >> "$PR_REVIEW_TEA_LOG" printf '%s\n' "$*" >> "$PR_REVIEW_TEA_LOG"
if [[ "$*" == "login list --output json" ]]; then if [[ "$*" == "login list --output json" ]]; then
printf '%s\n' '[{"name":"mosaicstack","url":"https://git.mosaicstack.dev"}]' printf '[{"name":"mosaicstack","url":"%s"}]\n' "$PR_REVIEW_LOGIN_URL"
exit 0 exit 0
fi fi
# tea v0.11.1 defines no --comment/-comment flag on `pr approve` or `pr echo "Unexpected tea command (wrapper must not write via tea): $*" >&2
# reject`; it fails closed with this exact message and a nonzero exit. Any exit 92
# regression that reintroduces the flag on those subcommands must hit this
# branch and fail RED (#835).
if [[ "$*" == *" -comment "* || "$*" == *" --comment "* || "$*" == *" -comment" || "$*" == *" --comment" ]]; then
echo "flag provided but not defined: -comment" >&2
exit 1
fi
case "${PR_REVIEW_TEST_MODE:-}" in
approve|paginated-approve|foreign-review)
[[ "$*" == "pr approve 123 --repo mosaicstack/stack --login mosaicstack" ]] || exit 90
;;
request-changes)
[[ "$*" == "pr reject 123 --repo mosaicstack/stack --login mosaicstack" ]] || exit 91
;;
legacy-fallback|comment-success|http-success|prefix-success|subpath-success|port-success|scp-ssh-success|url-ssh-success|ssh-transport-port-success|explicit-default-port-success|write-transport-failure|write-http-failure|readback-failure)
if [[ "$*" == pr\ comment* ]]; then
# tea v0.11.1 treats the nonexistent subcommand as `tea pr list` and exits 0.
printf '%s\n' 'INDEX TITLE STATE'
exit 0
fi
echo "Unexpected tea command: $*" >&2
exit 92
;;
*)
exit 95
;;
esac
SH SH
chmod +x "$BIN_DIR/tea" chmod +x "$BIN_DIR/tea"
# curl stub: a small REST server backed by persistent on-disk review/comment
# state.
cat > "$BIN_DIR/curl" <<'SH' cat > "$BIN_DIR/curl" <<'SH'
#!/usr/bin/env bash #!/usr/bin/env bash
set -euo pipefail set -euo pipefail
@@ -102,40 +98,19 @@ payload=""
url="" url=""
while [[ $# -gt 0 ]]; do while [[ $# -gt 0 ]]; do
case "$1" in case "$1" in
-o) -o) output_file="$2"; shift 2 ;;
output_file="$2" -w|-H) shift 2 ;;
shift 2 -X) method="$2"; shift 2 ;;
;; -d|--data) payload="$2"; shift 2 ;;
-w|-H) -s|-S|-sS) shift ;;
shift 2 http://*|https://*) url="$1"; shift ;;
;; *) shift ;;
-X)
method="$2"
shift 2
;;
-d|--data)
payload="$2"
shift 2
;;
-s|-S|-sS)
shift
;;
http://*|https://*)
url="$1"
shift
;;
*)
shift
;;
esac esac
done done
# Strip any query string so pagination params (?limit=&page=) don't defeat
# path matching, but keep the full URL in the log so tests can assert that the
# read-back paginated.
path="${url%%\?*}" path="${url%%\?*}"
page="${url##*page=}" query="${url#*\?}"
[[ "$page" == "$url" ]] && page=1 [[ "$query" == "$url" ]] && query=""
printf '%s %s\n' "$method" "$url" >> "$PR_REVIEW_CURL_LOG" printf '%s %s\n' "$method" "$url" >> "$PR_REVIEW_CURL_LOG"
write_response() { write_response() {
@@ -145,137 +120,202 @@ write_response() {
printf '%s' "$status" printf '%s' "$status"
} }
case "${PR_REVIEW_TEST_MODE:-}" in emit() {
legacy-fallback|write-transport-failure) # Split a two-line "status\n<json body>" python result into the response.
echo "simulated transport failure" >&2 local result="$1"
exit 7 write_response "$(printf '%s' "$result" | head -n1)" "$(printf '%s' "$result" | tail -n +2)"
;; }
write-http-failure)
write_response 500 '{"message":"simulated rejection"}' mode="${PR_REVIEW_TEST_MODE:-}"
;;
approve|request-changes|paginated-approve|foreign-review|comment-success|http-success|prefix-success|subpath-success|port-success|scp-ssh-success|url-ssh-success|ssh-transport-port-success|explicit-default-port-success|readback-failure) 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'
# Acting reviewer identity used for invocation attribution.
write_response 200 "$(PR_REVIEW_LOGIN="$PR_REVIEW_ACTING_LOGIN" 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"]}))
PY PY
)" )"
elif [[ "$method" == "GET" && "$path" == "$PR_REVIEW_EXPECTED_API_BASE/pulls/123/reviews" ]]; then elif [[ "$method" == "GET" && "$path" == "$PR_REVIEW_EXPECTED_API_BASE/pulls/123" ]]; then
# First (boundary) call precedes the write; later calls are the write_response 200 "$(PR_REVIEW_HEAD_SHA="$PR_REVIEW_HEAD_SHA" python3 - <<'PY'
# post-write read-back that must surface a strictly-newer review import json
# created by THIS action (id 200 > boundary 100), authored by the import os
# acting identity, with the expected state and pinned to the PR's print(json.dumps({"head": {"sha": os.environ["PR_REVIEW_HEAD_SHA"]}}))
# current head commit. The list is served PAGINATED so a target PY
# beyond page 1 is only found by a fully-paginating read-back. )"
calls_file="${PR_REVIEW_REVIEW_CALLS:-/dev/null}" elif [[ "$method" == "POST" && "$path" == "$PR_REVIEW_EXPECTED_API_BASE/pulls/123/reviews" ]]; then
if [[ -f "$calls_file" ]]; then printf '%s' "$payload" > "$PR_REVIEW_SUBMIT_PAYLOAD"
phase="post" emit "$(PR_REVIEW_PAYLOAD="$payload" python3 - <<'PY'
else
: > "$calls_file"
phase="boundary"
fi
state="APPROVED"
[[ "$PR_REVIEW_TEST_MODE" == "request-changes" ]] && state="REQUEST_CHANGES"
response=$(PR_REVIEW_PHASE="$phase" PR_REVIEW_PAGE="$page" \
PR_REVIEW_MODE="$PR_REVIEW_TEST_MODE" PR_REVIEW_STATE="$state" \
PR_REVIEW_ACTING_LOGIN="$PR_REVIEW_ACTING_LOGIN" \
PR_REVIEW_FOREIGN_LOGIN="$PR_REVIEW_FOREIGN_LOGIN" python3 - <<'PY'
import json import json
import os import os
phase = os.environ["PR_REVIEW_PHASE"] state_path = os.environ["PR_REVIEW_REVIEWS"]
page = int(os.environ["PR_REVIEW_PAGE"]) mode = os.environ["PR_REVIEW_TEST_MODE"]
mode = os.environ["PR_REVIEW_MODE"]
state = os.environ["PR_REVIEW_STATE"]
acting = os.environ["PR_REVIEW_ACTING_LOGIN"] acting = os.environ["PR_REVIEW_ACTING_LOGIN"]
foreign = os.environ["PR_REVIEW_FOREIGN_LOGIN"] foreign = os.environ["PR_REVIEW_FOREIGN_LOGIN"]
submitted = json.loads(os.environ["PR_REVIEW_PAYLOAD"])
with open(state_path, encoding="utf-8") as handle:
reviews = json.load(handle)
def review(review_id, review_state, commit, login): # no-op-concurrent-review: the wrapper's own submit is SUPPRESSED (200, no
return { # created object) even though a concurrent same-identity, same-state review at
"id": review_id, # the same head already exists. Nothing is persisted; no created id to verify.
"state": review_state, if mode == "no-op-concurrent-review":
"commit_id": commit, print("200")
"user": {"login": login}, print(json.dumps({}))
} raise SystemExit(0)
author = foreign if mode == "author-mismatch-review" else acting
new_id = (max((r["id"] for r in reviews), default=0)) + 1
record = {
"id": new_id,
"state": submitted.get("event"),
"commit_id": submitted.get("commit_id"),
"body": submitted.get("body"),
"user": {"login": author},
}
reviews.append(record)
with open(state_path, "w", encoding="utf-8") as handle:
json.dump(reviews, handle)
print("201")
print(json.dumps(record))
PY
)"
elif [[ "$method" == "GET" && "$path" == "$PR_REVIEW_EXPECTED_API_BASE"/pulls/123/reviews/* ]]; then
emit "$(PR_REVIEW_GET_ID="${path##*/}" python3 - <<'PY'
import json
import os
if phase == "boundary": state_path = os.environ["PR_REVIEW_REVIEWS"]
records = [review(100, "COMMENT", "oldsha0000", acting)] if page == 1 else [] wanted = int(os.environ["PR_REVIEW_GET_ID"])
elif mode == "paginated-approve": with open(state_path, encoding="utf-8") as handle:
# A full first page (50 non-matching records) forces the read-back to reviews = json.load(handle)
# request page 2, where the genuine matching review lives. match = next((r for r in reviews if r["id"] == wanted), None)
if page == 1: if match is None:
records = [review(101 + i, "COMMENT", "oldsha0000", acting) for i in range(50)] print("404")
elif page == 2: print(json.dumps({"message": "not found"}))
records = [review(200, state, "HEADSHA_FEEDFACE", acting)]
else:
records = []
elif mode == "foreign-review":
# A concurrent APPROVED review at the current head, but authored by a
# DIFFERENT identity. tea created nothing; attribution must reject this.
records = [review(200, state, "HEADSHA_FEEDFACE", foreign)] if page == 1 else []
else: else:
if page == 1: print("200")
records = [ print(json.dumps(match))
review(100, "COMMENT", "oldsha0000", acting),
review(200, state, "HEADSHA_FEEDFACE", acting),
]
else:
records = []
print(json.dumps(records))
PY PY
) )"
write_response 200 "$response" elif [[ "$method" == "GET" && "$path" == "$PR_REVIEW_EXPECTED_API_BASE/pulls/123/reviews" ]]; then
elif [[ "$method" == "GET" && "$path" == "$PR_REVIEW_EXPECTED_API_BASE/pulls/123" ]]; then emit "$(PR_REVIEW_QUERY="$query" python3 - <<'PY'
write_response 200 '{"head":{"sha":"HEADSHA_FEEDFACE"}}' import json
elif [[ "$method" == "POST" && "$url" == "$PR_REVIEW_EXPECTED_API_BASE/issues/123/comments" ]]; then import os
PR_REVIEW_PAYLOAD="$payload" python3 - <<'PY' from urllib.parse import parse_qs
state_path = os.environ["PR_REVIEW_REVIEWS"]
params = parse_qs(os.environ["PR_REVIEW_QUERY"])
limit = int(params.get("limit", ["50"])[0])
page = int(params.get("page", ["1"])[0])
with open(state_path, encoding="utf-8") as handle:
reviews = json.load(handle)
start = (page - 1) * limit
print("200")
print(json.dumps(reviews[start:start + limit]))
PY
)"
elif [[ "$method" == "POST" && "$path" == "$PR_REVIEW_EXPECTED_API_BASE/issues/123/comments" ]]; then
case "$mode" in
write-transport-failure)
echo "simulated transport failure" >&2
exit 7
;;
write-http-failure)
write_response 500 '{"message":"simulated rejection"}'
;;
*)
emit "$(PR_REVIEW_PAYLOAD="$payload" python3 - <<'PY'
import json import json
import os import os
assert json.loads(os.environ["PR_REVIEW_PAYLOAD"]) == {"body": os.environ["PR_REVIEW_EXPECTED_BODY"]} state_path = os.environ["PR_REVIEW_COMMENTS"]
PY acting = os.environ["PR_REVIEW_ACTING_LOGIN"]
response=$(python3 - <<'PY' base = os.environ["PR_REVIEW_EXPECTED_API_BASE"]
import json body = json.loads(os.environ["PR_REVIEW_PAYLOAD"]).get("body")
import os record = {
print(json.dumps({"id": 456, "body": os.environ["PR_REVIEW_EXPECTED_BODY"]}))
PY
)
write_response 201 "$response"
elif [[ "$method" == "GET" && "$url" == "$PR_REVIEW_EXPECTED_API_BASE/issues/comments/456" ]]; then
if [[ "$PR_REVIEW_TEST_MODE" == "readback-failure" ]]; then
body="different-body"
else
body="$PR_REVIEW_EXPECTED_BODY"
fi
response=$(PR_REVIEW_BODY="$body" python3 - <<'PY'
import json
import os
print(json.dumps({
"id": 456, "id": 456,
"body": os.environ["PR_REVIEW_BODY"], "body": body,
"issue_url": os.environ["PR_REVIEW_EXPECTED_API_BASE"] + "/issues/123", "user": {"login": acting},
})) "issue_url": f"{base}/issues/123",
}
with open(state_path, "w", encoding="utf-8") as handle:
json.dump([record], handle)
print("201")
print(json.dumps(record))
PY PY
) )"
write_response 200 "$response" ;;
else esac
echo "Unexpected curl request: $method $url" >&2 elif [[ "$method" == "GET" && "$path" == "$PR_REVIEW_EXPECTED_API_BASE"/issues/comments/* ]]; then
exit 97 emit "$(PR_REVIEW_GET_ID="${path##*/}" python3 - <<'PY'
fi import json
;; import os
*)
exit 98 state_path = os.environ["PR_REVIEW_COMMENTS"]
;; mode = os.environ["PR_REVIEW_TEST_MODE"]
esac wanted = int(os.environ["PR_REVIEW_GET_ID"])
with open(state_path, encoding="utf-8") as handle:
comments = json.load(handle)
match = next((c for c in comments if c["id"] == wanted), None)
if match is None:
print("404")
print(json.dumps({"message": "not found"}))
raise SystemExit(0)
if mode == "readback-failure":
# The server returns a DIFFERENT body than was created — a genuine
# provider-side mismatch the wrapper must reject.
match = dict(match, body="different-body")
print("200")
print(json.dumps(match))
PY
)"
else
echo "Unexpected curl request: $method $url" >&2
exit 97
fi
SH SH
chmod +x "$BIN_DIR/curl" chmod +x "$BIN_DIR/curl"
# Seed persistent server state for a mode before the wrapper runs.
seed_state() {
local mode="$1"
printf '[]' > "$COMMENTS_FILE"
rm -f "$SUBMIT_PAYLOAD_FILE"
PR_REVIEW_SEED_MODE="$mode" PR_REVIEW_SEED_ACTING="$ACTING_LOGIN" \
PR_REVIEW_SEED_HEAD="$HEAD_SHA" python3 - "$REVIEWS_FILE" <<'PY'
import json
import os
import sys
mode = os.environ["PR_REVIEW_SEED_MODE"]
acting = os.environ["PR_REVIEW_SEED_ACTING"]
head = os.environ["PR_REVIEW_SEED_HEAD"]
def review(rid, state, commit, login):
return {"id": rid, "state": state, "commit_id": commit, "user": {"login": login}}
if mode == "paginated-approve":
# 50 pre-existing reviews fill page 1 (limit 50); the review this run submits
# becomes id 51 and lands ALONE on page 2, exercising >page-1 pagination in
# the enumeration check.
reviews = [review(i, "COMMENT", "oldsha0000", acting) for i in range(1, 51)]
elif mode == "no-op-concurrent-review":
# A concurrent SAME-IDENTITY APPROVED review at the CURRENT head already
# exists. The wrapper's own submit will be a no-op; it must fail closed
# because no created id is returned — it must not scan and accept this one.
reviews = [review(77, "APPROVED", head, acting)]
else:
reviews = [review(100, "COMMENT", "oldsha0000", acting)]
with open(sys.argv[1], "w", encoding="utf-8") as handle:
json.dump(reviews, handle)
PY
}
run_review() { run_review() {
local mode="$1" action="$2" comment="${3:-}" local mode="$1" action="$2" comment="${3:-}"
local configured_url="${4:-https://git.mosaicstack.dev}" local configured_url="${4:-https://git.mosaicstack.dev}"
@@ -288,101 +328,144 @@ run_review() {
: > "$TEA_LOG" : > "$TEA_LOG"
: > "$CURL_LOG" : > "$CURL_LOG"
: > "$OUTPUT_FILE" : > "$OUTPUT_FILE"
rm -f "$WORK_DIR/review_calls" seed_state "$mode"
( (
cd "$REPO_DIR" cd "$REPO_DIR"
PATH="$BIN_DIR:$PATH" \ PATH="$BIN_DIR:$PATH" \
XDG_CONFIG_HOME="$XDG_DIR" \
MOSAIC_CREDENTIALS_FILE="$CREDENTIALS_FILE" \ MOSAIC_CREDENTIALS_FILE="$CREDENTIALS_FILE" \
PR_REVIEW_TEA_LOG="$TEA_LOG" \ PR_REVIEW_TEA_LOG="$TEA_LOG" \
PR_REVIEW_LOGIN_URL="${configured_url%/}" \
PR_REVIEW_CURL_LOG="$CURL_LOG" \ PR_REVIEW_CURL_LOG="$CURL_LOG" \
PR_REVIEW_REVIEW_CALLS="$WORK_DIR/review_calls" \ PR_REVIEW_REVIEWS="$REVIEWS_FILE" \
PR_REVIEW_COMMENTS="$COMMENTS_FILE" \
PR_REVIEW_SUBMIT_PAYLOAD="$SUBMIT_PAYLOAD_FILE" \
PR_REVIEW_TEST_MODE="$mode" \ PR_REVIEW_TEST_MODE="$mode" \
PR_REVIEW_EXPECTED_BODY="$comment" \ PR_REVIEW_EXPECTED_BODY="$comment" \
PR_REVIEW_EXPECTED_API_BASE="$expected_api_base" \ PR_REVIEW_EXPECTED_API_BASE="$expected_api_base" \
PR_REVIEW_API_ROOT="$expected_api_root" \ PR_REVIEW_API_ROOT="$expected_api_root" \
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"} "$SCRIPT_DIR/pr-review.sh" -n 123 -a "$action" ${comment:+-c "$comment"}
) > "$OUTPUT_FILE" 2>&1 ) > "$OUTPUT_FILE" 2>&1
} }
assert_no_tea_write() {
# tea must only ever be used for the login list, never to write.
if grep -qvE '^login list --output json$' "$TEA_LOG"; then
echo "FAIL: wrapper invoked tea for something other than the login list" >&2
cat "$TEA_LOG" >&2
exit 1
fi
}
# Case 1: a plain approve submits a review via REST and verifies it by its exact
# provider-returned id (id 101), attributed to the acting identity, pinned to
# the PR head, with no separate comment.
run_review approve approve run_review approve approve
grep -q '^pr approve 123 --repo mosaicstack/stack --login mosaicstack$' "$TEA_LOG" grep -q 'Approved and verified Gitea PR #123 (review ID 101)' "$OUTPUT_FILE"
grep -q 'Approved and verified Gitea PR #123 (review ID 200)' "$OUTPUT_FILE"
# #865: the approval STATE itself is read back — a pre-write boundary GET and a
# post-write read-back GET on the (paginated) reviews endpoint, plus a PR head
# lookup and an acting-identity (GET /user) resolution for attribution.
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$' "$CURL_LOG"
grep -q '^GET https://git.mosaicstack.dev/api/v1/user$' "$CURL_LOG" grep -q '^GET https://git.mosaicstack.dev/api/v1/user$' "$CURL_LOG"
if grep -q 'comment' "$TEA_LOG"; then grep -q '^GET https://git.mosaicstack.dev/api/v1/repos/mosaicstack/stack/pulls/123$' "$CURL_LOG"
echo "Plain approve (no review body) unexpectedly touched comment persistence" >&2 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"
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'
import json
import os
import sys
payload = json.load(open(sys.argv[1], encoding="utf-8"))
assert payload["event"] == "APPROVED", payload
assert payload["commit_id"] == os.environ["PR_REVIEW_HEAD_SHA"], payload
PY
# A plain approve (no body) must not POST a comment.
if grep -q '/issues/123/comments' "$CURL_LOG"; then
echo "FAIL: plain approve unexpectedly posted a comment" >&2
exit 1 exit 1
fi fi
# #865 (invocation attribution): a concurrent APPROVED review at the current # Case 2: a submitted review NOT authored by the acting identity must FAIL
# head, authored by a DIFFERENT identity while tea created nothing, must NOT # CLOSED — the exact-id read-back enforces authorship.
# satisfy verification — id-above-boundary + state + head is only temporal if run_review author-mismatch-review approve; then
# ordering, not proof THIS reviewer wrote it.
if run_review foreign-review approve; then
echo "FAIL: approve accepted a review authored by a different identity" >&2 echo "FAIL: approve accepted a review authored by a different identity" >&2
cat "$OUTPUT_FILE" >&2 cat "$OUTPUT_FILE" >&2
exit 1 exit 1
fi fi
if grep -q 'Approved and verified' "$OUTPUT_FILE"; then if grep -q 'Approved and verified' "$OUTPUT_FILE"; then
echo "FAIL: read-back matched a different-identity review (attribution bypassed)" >&2 echo "FAIL: read-back did not enforce acting-identity authorship" >&2
exit 1 exit 1
fi fi
# #865 (pagination): a genuine matching review that lands beyond page 1 of the # Case 3: a no-op submit with a concurrent SAME-IDENTITY, same-state review at
# reviews list must still be found by a fully-paginating read-back. # the current head already present must FAIL CLOSED — the closed concurrency
run_review paginated-approve approve # window. The wrapper must not read back (or accept) the concurrent id 77.
grep -q 'Approved and verified Gitea PR #123 (review ID 200)' "$OUTPUT_FILE" if run_review no-op-concurrent-review approve; then
grep -q '^GET https://git.mosaicstack.dev/api/v1/repos/mosaicstack/stack/pulls/123/reviews?limit=[0-9]*&page=2$' "$CURL_LOG" echo "FAIL: approve reported success when its submit no-opped but a concurrent review existed" >&2
# #835: tea v0.11.1 defines no --comment/-comment flag on `pr approve`. A
# review body supplied alongside approve must be routed through the durable
# comment REST API instead of being passed to `tea` directly.
run_review approve approve approve-note
grep -q '^pr approve 123 --repo mosaicstack/stack --login mosaicstack$' "$TEA_LOG"
grep -q 'Approved and verified Gitea PR #123 (review ID 200)' "$OUTPUT_FILE"
grep -q '^POST https://git.mosaicstack.dev/api/v1/repos/mosaicstack/stack/issues/123/comments$' "$CURL_LOG"
grep -q '^GET https://git.mosaicstack.dev/api/v1/repos/mosaicstack/stack/issues/comments/456$' "$CURL_LOG"
grep -q 'Added and verified review comment on Gitea PR #123 (comment ID 456)' "$OUTPUT_FILE"
# #835: same for `pr reject` (request-changes), where a comment is required.
run_review request-changes request-changes changes-required
grep -q '^pr reject 123 --repo mosaicstack/stack --login mosaicstack$' "$TEA_LOG"
grep -q 'Requested changes and verified on Gitea PR #123 (review ID 200)' "$OUTPUT_FILE"
grep -q '^GET https://git.mosaicstack.dev/api/v1/repos/mosaicstack/stack/pulls/123/reviews?limit=[0-9]*&page=1$' "$CURL_LOG"
grep -q '^POST https://git.mosaicstack.dev/api/v1/repos/mosaicstack/stack/issues/123/comments$' "$CURL_LOG"
grep -q '^GET https://git.mosaicstack.dev/api/v1/repos/mosaicstack/stack/issues/comments/456$' "$CURL_LOG"
grep -q 'Added and verified review comment on Gitea PR #123 (comment ID 456)' "$OUTPUT_FILE"
if run_review legacy-fallback comment durable-body; then
echo "The old nonexistent tea pr comment fallback returned success" >&2
cat "$OUTPUT_FILE" >&2 cat "$OUTPUT_FILE" >&2
exit 1 exit 1
fi fi
if grep -q '^pr comment ' "$TEA_LOG"; then if grep -q 'Approved and verified' "$OUTPUT_FILE"; then
echo "Wrapper invoked unsupported tea pr comment" >&2 echo "FAIL: approve accepted a concurrent review for a no-op submit (window not closed)" >&2
exit 1 exit 1
fi fi
if grep -q 'Added comment to Gitea PR' "$OUTPUT_FILE"; then if grep -q '/pulls/123/reviews/77$' "$CURL_LOG"; then
echo "Wrapper reported success without durable persistence" >&2 echo "FAIL: wrapper read back the concurrent review id 77 (illegitimate fallback)" >&2
exit 1 exit 1
fi fi
# Case 4: a genuine matching review that lands beyond page 1 of the reviews list
# must still be found by the fully-paginating enumeration check.
run_review paginated-approve approve
grep -q 'Approved and verified Gitea PR #123 (review ID 51)' "$OUTPUT_FILE"
grep -q '^GET https://git.mosaicstack.dev/api/v1/repos/mosaicstack/stack/pulls/123/reviews/51$' "$CURL_LOG"
grep -q '^GET https://git.mosaicstack.dev/api/v1/repos/mosaicstack/stack/pulls/123/reviews?limit=[0-9]*&page=2$' "$CURL_LOG"
# Case 5: an approve WITH a body carries that body in the review submit itself —
# there is no separate detached comment POST.
run_review approve approve approve-note
grep -q 'Approved and verified Gitea PR #123 (review ID 101)' "$OUTPUT_FILE"
PR_REVIEW_EXPECTED_BODY="approve-note" python3 - "$SUBMIT_PAYLOAD_FILE" <<'PY'
import json
import os
import sys
payload = json.load(open(sys.argv[1], encoding="utf-8"))
assert payload["body"] == os.environ["PR_REVIEW_EXPECTED_BODY"], payload
PY
if grep -q '/issues/123/comments' "$CURL_LOG"; then
echo "FAIL: approve-with-body posted a separate comment instead of carrying the body on the review" >&2
exit 1
fi
# Case 6: request-changes requires a body and carries it on the REQUEST_CHANGES
# review submit.
run_review request-changes request-changes changes-required
grep -q 'Requested changes and verified on Gitea PR #123 (review ID 101)' "$OUTPUT_FILE"
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"
PR_REVIEW_EXPECTED_BODY="changes-required" python3 - "$SUBMIT_PAYLOAD_FILE" <<'PY'
import json
import os
import sys
payload = json.load(open(sys.argv[1], encoding="utf-8"))
assert payload["event"] == "REQUEST_CHANGES", payload
assert payload["body"] == os.environ["PR_REVIEW_EXPECTED_BODY"], payload
PY
assert_no_tea_write
# Case 7: the `comment` action creates a comment via REST and verifies it by its
# exact created id, attributed to the acting identity. This also exercises
# owner/repo + base-URL resolution across clone-URL shapes.
complex_body=$'durable "body"\n-- marker' complex_body=$'durable "body"\n-- marker'
run_review comment-success comment "$complex_body" run_review comment-success comment "$complex_body"
grep -q '^POST https://git.mosaicstack.dev/api/v1/repos/mosaicstack/stack/issues/123/comments$' "$CURL_LOG" grep -q '^POST https://git.mosaicstack.dev/api/v1/repos/mosaicstack/stack/issues/123/comments$' "$CURL_LOG"
grep -q '^GET https://git.mosaicstack.dev/api/v1/repos/mosaicstack/stack/issues/comments/456$' "$CURL_LOG" grep -q '^GET https://git.mosaicstack.dev/api/v1/repos/mosaicstack/stack/issues/comments/456$' "$CURL_LOG"
grep -q 'Added and verified comment on Gitea PR #123' "$OUTPUT_FILE" grep -q 'Added and verified comment on Gitea PR #123' "$OUTPUT_FILE"
if [[ -s "$TEA_LOG" ]]; then assert_no_tea_write
echo "REST comment path unexpectedly invoked tea" >&2
cat "$TEA_LOG" >&2
exit 1
fi
run_review http-success comment durable-body http://git.mosaicstack.dev run_review http-success comment durable-body http://git.mosaicstack.dev
grep -q '^POST http://git.mosaicstack.dev/api/v1/repos/mosaicstack/stack/issues/123/comments$' "$CURL_LOG" grep -q '^POST http://git.mosaicstack.dev/api/v1/repos/mosaicstack/stack/issues/123/comments$' "$CURL_LOG"
@@ -410,23 +493,17 @@ grep -q '^POST https://git.example/api/v1/repos/owner/repo/issues/123/comments$'
run_review url-ssh-success comment durable-body https://git.example ssh://git@git.example/owner/repo.git owner/repo run_review url-ssh-success comment durable-body https://git.example ssh://git@git.example/owner/repo.git owner/repo
grep -q '^POST https://git.example/api/v1/repos/owner/repo/issues/123/comments$' "$CURL_LOG" grep -q '^POST https://git.example/api/v1/repos/owner/repo/issues/123/comments$' "$CURL_LOG"
# #850 (follow-up to #812): an SSH remote's transport port (e.g. `ssh:// # #850: an SSH remote's transport port must not be compared against the
# git@host:2222/...`) must NOT be compared against the configured HTTP(S) API # configured HTTP(S) API URL's port.
# URL's port -- they identify unrelated properties (SSH daemon port vs. HTTP(S)
# provider port) of the same Gitea host. Before the fix, host-match required
# the configured URL to carry the identical port, so this failed closed even
# though both remote and configured URL name the same host.
run_review ssh-transport-port-success comment durable-body https://git.example ssh://git@git.example:2222/owner/repo.git owner/repo run_review ssh-transport-port-success comment durable-body https://git.example ssh://git@git.example:2222/owner/repo.git owner/repo
grep -q '^POST https://git.example/api/v1/repos/owner/repo/issues/123/comments$' "$CURL_LOG" grep -q '^POST https://git.example/api/v1/repos/owner/repo/issues/123/comments$' "$CURL_LOG"
# #850 (follow-up to #812): an explicit default HTTP(S) port on the remote # #850: an explicit default HTTP(S) port on the remote must equal an implicit
# (`https://host:443/...`) must be treated as equal to an implicit # (portless) configured URL.
# (portless) configured URL on BOTH sides -- the pre-fix comparison only
# normalized the default port when the REMOTE side was portless, so the
# inverse (explicit remote, implicit configured) form failed closed.
run_review explicit-default-port-success comment durable-body https://git.example https://git.example:443/owner/repo.git owner/repo run_review explicit-default-port-success comment durable-body https://git.example https://git.example:443/owner/repo.git owner/repo
grep -q '^POST https://git.example/api/v1/repos/owner/repo/issues/123/comments$' "$CURL_LOG" grep -q '^POST https://git.example/api/v1/repos/owner/repo/issues/123/comments$' "$CURL_LOG"
# Comment write/read-back failure modes must all fail closed.
if run_review write-transport-failure comment durable-body; then if run_review write-transport-failure comment durable-body; then
echo "Expected provider transport failure to return nonzero" >&2 echo "Expected provider transport failure to return nonzero" >&2
exit 1 exit 1
@@ -444,4 +521,4 @@ if grep -q 'Added and verified comment' "$OUTPUT_FILE"; then
exit 1 exit 1
fi fi
echo "pr-review.sh durable Gitea comment regression passed" echo "pr-review.sh REST review + comment create/read-back regression passed"