From ce6d735128ba0f97b634e80465485404a74af6b0 Mon Sep 17 00:00:00 2001 From: coder3 Date: Wed, 12 Aug 2026 21:08:34 -0500 Subject: [PATCH] fix(git-tools): bind PR edits to acting identity (#1080) --- .../mosaic/framework/tools/git/pr-edit.sh | 158 ++++++++++-------- .../framework/tools/git/test-pr-edit.sh | 149 +++++++++-------- 2 files changed, 172 insertions(+), 135 deletions(-) diff --git a/packages/mosaic/framework/tools/git/pr-edit.sh b/packages/mosaic/framework/tools/git/pr-edit.sh index 42133724..d91e97bc 100755 --- a/packages/mosaic/framework/tools/git/pr-edit.sh +++ b/packages/mosaic/framework/tools/git/pr-edit.sh @@ -1,6 +1,6 @@ #!/usr/bin/env bash # pr-edit.sh - Edit a pull request on GitHub or Gitea -# Usage: pr-edit.sh -n [-t ] [-b <body>] [-B <base>] [--draft|--ready] [--login <name>] +# Usage: pr-edit.sh -n <pr_number> [-t <title>] [-b <body>] [-B <base>] [--draft|--ready] [--login <name>] [-r owner/repo] [-H host] set -euo pipefail @@ -14,6 +14,14 @@ BODY="" BASE_BRANCH="" DRAFT_MODE="" LOGIN_OVERRIDE="" +REPO_OVERRIDE="" +HOST_OVERRIDE="" +AUTH_CONFIG="" + +cleanup() { + [[ -z "$AUTH_CONFIG" ]] || rm -f -- "$AUTH_CONFIG" +} +trap cleanup EXIT HUP INT TERM usage() { cat <<EOF @@ -28,7 +36,9 @@ Options: -B, --base BRANCH New base branch --draft Mark the pull request as draft --ready Mark the pull request ready for review - --login NAME Gitea tea login (must match the repository host) + -l, --login NAME Gitea login (must authenticate as MOSAIC_GIT_IDENTITY) + -r, --repo OWNER/REPO Explicit target repository + -H, --host HOST Explicit Gitea host (required with --repo off-host) -h, --help Show this help message EOF exit "${1:-1}" @@ -36,43 +46,21 @@ EOF while [[ $# -gt 0 ]]; do case "$1" in - -n|--number) - PR_NUMBER="${2:-}" - shift 2 - ;; - -t|--title) - TITLE="${2:-}" - shift 2 - ;; - -b|--body) - BODY="${2:-}" - shift 2 - ;; - -B|--base) - BASE_BRANCH="${2:-}" - shift 2 - ;; + -n|--number) PR_NUMBER="${2:-}"; shift 2 ;; + -t|--title) TITLE="${2:-}"; shift 2 ;; + -b|--body) BODY="${2:-}"; shift 2 ;; + -B|--base) BASE_BRANCH="${2:-}"; shift 2 ;; --draft) [[ "$DRAFT_MODE" != "ready" ]] || { echo "Error: --draft and --ready are mutually exclusive" >&2; exit 1; } - DRAFT_MODE="draft" - shift - ;; + DRAFT_MODE="draft"; shift ;; --ready) [[ "$DRAFT_MODE" != "draft" ]] || { echo "Error: --draft and --ready are mutually exclusive" >&2; exit 1; } - DRAFT_MODE="ready" - shift - ;; - --login) - LOGIN_OVERRIDE="${2:-}" - shift 2 - ;; - -h|--help) - usage 0 - ;; - *) - echo "Unknown option: $1" >&2 - usage - ;; + DRAFT_MODE="ready"; shift ;; + -l|--login) LOGIN_OVERRIDE="${2:-}"; shift 2 ;; + -r|--repo) REPO_OVERRIDE="${2:-}"; shift 2 ;; + -H|--host) HOST_OVERRIDE="${2:-}"; shift 2 ;; + -h|--help) usage 0 ;; + *) echo "Unknown option: $1" >&2; usage ;; esac done @@ -82,8 +70,16 @@ if [[ -z "$TITLE" && -z "$BODY" && -z "$BASE_BRANCH" && -z "$DRAFT_MODE" ]]; the echo "Error: At least one edit option is required" >&2 exit 1 fi +[[ -z "$REPO_OVERRIDE" || "$REPO_OVERRIDE" =~ ^[^/[:space:]]+/[^/[:space:]]+$ ]] || { + echo "Error: --repo must be OWNER/REPO" >&2 + exit 1 +} -PLATFORM=$(detect_platform) +if [[ -n "$HOST_OVERRIDE" || -n "$REPO_OVERRIDE" ]]; then + PLATFORM="gitea" +else + PLATFORM=$(detect_platform) +fi case "$PLATFORM" in github) @@ -102,44 +98,74 @@ case "$PLATFORM" in fi ;; gitea) - HOST=$(get_remote_host) || { echo "Error: Could not resolve Gitea host from remote" >&2; exit 1; } - REPO_SLUG=$(get_repo_slug) || { echo "Error: Could not resolve Gitea repo slug from remote" >&2; exit 1; } - if [[ -n "$LOGIN_OVERRIDE" ]]; then - GITEA_LOGIN="$LOGIN_OVERRIDE" - else - GITEA_LOGIN=$(get_gitea_login) || { echo "Error: Could not resolve Gitea login for remote host" >&2; exit 1; } - fi - # Bind the credential to the explicitly selected login and repository host. - TOKEN=$(get_gitea_token_for_login "$GITEA_LOGIN" "$HOST") || { - echo "Error: Could not resolve token for Gitea login '$GITEA_LOGIN' on '$HOST'" >&2 + IDENTITY="${MOSAIC_GIT_IDENTITY:-}" + [[ -n "$IDENTITY" ]] || { + echo "Error: MOSAIC_GIT_IDENTITY is required for a mutating Gitea operation" >&2 exit 1 } + HOST="${HOST_OVERRIDE:-}" + if [[ -z "$HOST" ]]; then + HOST=$(get_remote_host) || { + echo "Error: Could not resolve Gitea host; pass --host with --repo" >&2 + exit 1 + } + fi + HOST="${HOST#http://}"; HOST="${HOST#https://}"; HOST="${HOST%%/*}" + REPO_SLUG="${REPO_OVERRIDE:-}" + if [[ -z "$REPO_SLUG" ]]; then + REPO_SLUG=$(get_repo_slug) || { echo "Error: Could not resolve Gitea repo slug from remote" >&2; exit 1; } + fi + + if [[ -n "$LOGIN_OVERRIDE" ]]; then + GITEA_LOGIN_NAME="$LOGIN_OVERRIDE" + elif [[ -n "${GITEA_LOGIN:-}" ]]; then + GITEA_LOGIN_NAME="$GITEA_LOGIN" + else + echo "Error: --login (or GITEA_LOGIN) is required; refusing host-first login selection" >&2 + exit 1 + fi + + TOKEN=$(get_gitea_token_for_login "$GITEA_LOGIN_NAME" "$HOST") || { + echo "Error: login '$GITEA_LOGIN_NAME' is not configured for target host '$HOST'" >&2 + exit 1 + } + AUTHENTICATED_USER=$(get_gitea_authenticated_user "$GITEA_LOGIN_NAME") || { + echo "Error: could not authenticate Gitea login '$GITEA_LOGIN_NAME'" >&2 + exit 1 + } + [[ "$AUTHENTICATED_USER" == "$IDENTITY" ]] || { + echo "Error: Gitea login '$GITEA_LOGIN_NAME' authenticates as '$AUTHENTICATED_USER', not MOSAIC_GIT_IDENTITY '$IDENTITY'" >&2 + exit 1 + } + AUTH_CONFIG=$(gitea_write_auth_config "$TOKEN") || { + echo "Error: could not stage private Gitea authentication" >&2 + exit 1 + } + unset TOKEN + + API_BASE="https://${HOST}/api/v1/repos/${REPO_SLUG}" + # Preflight the explicit host/repo pair before any mutation. This prevents + # a slug inferred from one checkout being combined with another host. + curl -fsS --config "$AUTH_CONFIG" -H "User-Agent: mosaic-pr-edit" "$API_BASE" >/dev/null || { + echo "Error: target repository preflight failed for https://${HOST}/${REPO_SLUG}" >&2 + exit 1 + } + PAYLOAD=$(TITLE="$TITLE" BODY="$BODY" BASE_BRANCH="$BASE_BRANCH" DRAFT_MODE="$DRAFT_MODE" python3 - <<'PY' import json import os - payload = {} -if os.environ["TITLE"]: - payload["title"] = os.environ["TITLE"] -if os.environ["BODY"]: - payload["body"] = os.environ["BODY"] -if os.environ["BASE_BRANCH"]: - payload["base"] = os.environ["BASE_BRANCH"] -if os.environ["DRAFT_MODE"]: - payload["draft"] = os.environ["DRAFT_MODE"] == "draft" +if os.environ["TITLE"]: payload["title"] = os.environ["TITLE"] +if os.environ["BODY"]: payload["body"] = os.environ["BODY"] +if os.environ["BASE_BRANCH"]: payload["base"] = os.environ["BASE_BRANCH"] +if os.environ["DRAFT_MODE"]: payload["draft"] = os.environ["DRAFT_MODE"] == "draft" print(json.dumps(payload)) PY ) - curl -fsS -X PATCH \ - -H "User-Agent: mosaic-pr-edit" \ - -H "Authorization: token ${TOKEN}" \ - -H "Content-Type: application/json" \ - -d "$PAYLOAD" \ - "https://${HOST}/api/v1/repos/${REPO_SLUG}/pulls/${PR_NUMBER}" - echo "Updated Gitea pull request #$PR_NUMBER using login '$GITEA_LOGIN'" >&2 - ;; - *) - echo "Error: Could not detect git platform" >&2 - exit 1 + curl -fsS --config "$AUTH_CONFIG" -X PATCH \ + -H "User-Agent: mosaic-pr-edit" -H "Content-Type: application/json" \ + -d "$PAYLOAD" "$API_BASE/pulls/${PR_NUMBER}" + echo "Updated Gitea pull request #$PR_NUMBER as '$AUTHENTICATED_USER'" >&2 ;; + *) echo "Error: Could not detect git platform" >&2; exit 1 ;; esac diff --git a/packages/mosaic/framework/tools/git/test-pr-edit.sh b/packages/mosaic/framework/tools/git/test-pr-edit.sh index 3a8f9a6d..a915b7bc 100755 --- a/packages/mosaic/framework/tools/git/test-pr-edit.sh +++ b/packages/mosaic/framework/tools/git/test-pr-edit.sh @@ -1,112 +1,123 @@ #!/usr/bin/env bash -# Regression harness for PR editing, including Gitea draft/ready and login binding. +# Regression harness for secret-safe, identity-bound PR editing and explicit targets. set -euo pipefail SCRIPT_DIR="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)" WORK_DIR="${MOSAIC_TEST_WORK_DIR:-$PWD/.mosaic-test-work/pr-edit}" -REPO_DIR="$WORK_DIR/repo" -BIN_DIR="$WORK_DIR/bin" -HOME_DIR="$WORK_DIR/home" -XDG_DIR="$WORK_DIR/xdg" -LOG_FILE="$WORK_DIR/calls.log" - -rm -rf "$WORK_DIR" -mkdir -p "$REPO_DIR" "$BIN_DIR" "$HOME_DIR" "$XDG_DIR/tea" +REPO_DIR="$WORK_DIR/repo"; BIN_DIR="$WORK_DIR/bin"; HOME_DIR="$WORK_DIR/home" +XDG_DIR="$WORK_DIR/xdg"; LOG_FILE="$WORK_DIR/calls.log" +rm -rf "$WORK_DIR"; mkdir -p "$REPO_DIR" "$BIN_DIR" "$HOME_DIR" "$XDG_DIR/tea" git -C "$REPO_DIR" init -q -git -C "$REPO_DIR" remote add origin https://git.uscllc.com/USC/uconnect.git +git -C "$REPO_DIR" remote add origin https://git.uscllc.com/other/wrong-checkout.git git -C "$REPO_DIR" config mosaic.gitIdentity "" cat > "$XDG_DIR/tea/config.yml" <<'YAML' logins: -- name: usc +- name: usc-coder3 url: https://git.uscllc.com token: fixture-usc-token -- name: mosaicstack +- name: same-host-other + url: https://git.uscllc.com + token: fixture-other-token +- name: mosaic-coder3 url: https://git.mosaicstack.dev token: fixture-mosaic-token YAML +cat > "$BIN_DIR/tea" <<'SH' +#!/usr/bin/env bash +set -euo pipefail +[[ "$*" == "api --login usc-coder3 /user" ]] && { printf '{"login":"coder3"}\n'; exit 0; } +[[ "$*" == "api --login same-host-other /user" ]] && { printf '{"login":"other"}\n'; exit 0; } +[[ "$*" == "api --login mosaic-coder3 /user" ]] && { printf '{"login":"coder3"}\n'; exit 0; } +exit 1 +SH cat > "$BIN_DIR/curl" <<'SH' #!/usr/bin/env bash set -euo pipefail -printf 'curl' >> "$MOSAIC_TEST_LOG" -printf ' <%s>' "$@" >> "$MOSAIC_TEST_LOG" -printf '\n' >> "$MOSAIC_TEST_LOG" -printf '{"number":42,"draft":false}\n' +printf 'curl' >> "$MOSAIC_TEST_LOG"; printf ' <%s>' "$@" >> "$MOSAIC_TEST_LOG"; printf '\n' >> "$MOSAIC_TEST_LOG" +[[ " $* " == *" -X PATCH "* ]] && printf '{"number":42,"draft":false}\n' || printf '{"name":"repo"}\n' SH - cat > "$BIN_DIR/gh" <<'SH' #!/usr/bin/env bash set -euo pipefail -printf 'gh' >> "$MOSAIC_TEST_LOG" -printf ' <%s>' "$@" >> "$MOSAIC_TEST_LOG" -printf '\n' >> "$MOSAIC_TEST_LOG" +printf 'gh' >> "$MOSAIC_TEST_LOG"; printf ' <%s>' "$@" >> "$MOSAIC_TEST_LOG"; printf '\n' >> "$MOSAIC_TEST_LOG" SH -chmod +x "$BIN_DIR/curl" "$BIN_DIR/gh" "$SCRIPT_DIR/pr-edit.sh" +chmod +x "$BIN_DIR/tea" "$BIN_DIR/curl" "$BIN_DIR/gh" "$SCRIPT_DIR/pr-edit.sh" run_wrapper() { - ( - cd "$REPO_DIR" - PATH="$BIN_DIR:$PATH" \ - HOME="$HOME_DIR" \ - XDG_CONFIG_HOME="$XDG_DIR" \ - MOSAIC_TEST_LOG="$LOG_FILE" \ - "$SCRIPT_DIR/pr-edit.sh" "$@" - ) + (cd "$REPO_DIR"; PATH="$BIN_DIR:$PATH" HOME="$HOME_DIR" XDG_CONFIG_HOME="$XDG_DIR" \ + MOSAIC_TEST_LOG="$LOG_FILE" "$SCRIPT_DIR/pr-edit.sh" "$@") +} +assert_no_secret() { + ! grep -q 'fixture-.*-token' "$LOG_FILE" || { echo "Credential leaked into curl argv/log" >&2; exit 1; } } +# The explicit target differs from CWD origin and must govern BOTH host and slug. : > "$LOG_FILE" # shellcheck disable=SC2016 # literal backticks prove argument-array body safety. -run_wrapper -n 42 --login usc --title 'New title' --body 'Body with `literal` bytes' --base develop --draft >/dev/null +MOSAIC_GIT_IDENTITY=coder3 run_wrapper -n 42 --login mosaic-coder3 -r mosaicstack/stack \ + -H git.mosaicstack.dev --title 'New title' --body 'Body with `literal` bytes' --base develop --draft >/dev/null python3 - "$LOG_FILE" <<'PY' -import json -import pathlib -import sys - -line = pathlib.Path(sys.argv[1]).read_text() -assert "-X> <PATCH>" in line, line -assert "Authorization: token fixture-usc-token" in line, line -assert "https://git.uscllc.com/api/v1/repos/USC/uconnect/pulls/42" in line, line -payload = line.split(" <-d> <", 1)[1].split("> <https://", 1)[0] -data = json.loads(payload) -assert data == { - "title": "New title", - "body": "Body with `literal` bytes", - "base": "develop", - "draft": True, -}, data +import json, pathlib, sys +lines = pathlib.Path(sys.argv[1]).read_text().splitlines() +assert len(lines) == 2, lines +assert "https://git.mosaicstack.dev/api/v1/repos/mosaicstack/stack" in lines[0], lines +assert "https://git.mosaicstack.dev/api/v1/repos/mosaicstack/stack/pulls/42" in lines[1], lines +assert "--config" in lines[0] and "--config" in lines[1], lines +assert "Authorization:" not in "\n".join(lines), lines +payload = lines[1].split(" <-d> <", 1)[1].split("> <https://", 1)[0] +assert json.loads(payload) == {"title":"New title","body":"Body with `literal` bytes","base":"develop","draft":True} PY +assert_no_secret +# Ready maps to false and still preflights before the write. : > "$LOG_FILE" -run_wrapper -n 42 --login usc --ready >/dev/null -grep -q '"draft": false' "$LOG_FILE" +MOSAIC_GIT_IDENTITY=coder3 run_wrapper -n 42 --login usc-coder3 -r USC/uconnect -H git.uscllc.com --ready >/dev/null +grep -q '"draft": false' "$LOG_FILE"; assert_no_secret +# Identity is mandatory; no ambient/first-host login can write. : > "$LOG_FILE" -if run_wrapper -n 42 --login mosaicstack --draft >/dev/null 2>&1; then - echo "Expected a login bound to another host to fail closed" >&2 - exit 1 -fi -[[ ! -s "$LOG_FILE" ]] || { echo "Mismatched login unexpectedly reached curl" >&2; exit 1; } +if run_wrapper -n 42 --login usc-coder3 --draft >/dev/null 2>&1; then echo "Unset identity wrote" >&2; exit 1; fi +[[ ! -s "$LOG_FILE" ]] || { echo "Unset identity reached curl" >&2; exit 1; } -if run_wrapper -n 42 --draft --ready >/dev/null 2>&1; then - echo "Expected --draft and --ready to be mutually exclusive" >&2 - exit 1 -fi -if run_wrapper -n 42 >/dev/null 2>&1; then - echo "Expected a no-op edit to fail" >&2 - exit 1 -fi -if ! run_wrapper --help 2>&1 | grep -q '^Usage:'; then - echo "Expected --help to exit zero and print usage" >&2 - exit 1 -fi +# Explicit and ambient same-host wrong principals both refuse before preflight/write. +for mode in explicit ambient; do + : > "$LOG_FILE" + if [[ "$mode" == explicit ]]; then + cmd=(--login same-host-other) + else + cmd=(); export GITEA_LOGIN=same-host-other + fi + if MOSAIC_GIT_IDENTITY=coder3 run_wrapper -n 42 "${cmd[@]}" -r USC/uconnect -H git.uscllc.com --draft >/dev/null 2>&1; then + echo "$mode wrong identity wrote" >&2; exit 1 + fi + unset GITEA_LOGIN + [[ ! -s "$LOG_FILE" ]] || { echo "$mode wrong identity reached curl" >&2; exit 1; } +done -# GitHub uses gh's supported edit and ready/undo commands rather than raw defaults. +# Set identity with no explicit/ambient login refuses rather than selecting first host login. +: > "$LOG_FILE" +if MOSAIC_GIT_IDENTITY=coder3 run_wrapper -n 42 -r USC/uconnect -H git.uscllc.com --draft >/dev/null 2>&1; then + echo "Missing login selected a principal" >&2; exit 1 +fi +[[ ! -s "$LOG_FILE" ]] || { echo "Missing login reached curl" >&2; exit 1; } + +# Cross-host credential fails before curl; explicit target preflight failure blocks PATCH. +: > "$LOG_FILE" +if MOSAIC_GIT_IDENTITY=coder3 run_wrapper -n 42 --login mosaic-coder3 -r USC/uconnect -H git.uscllc.com --draft >/dev/null 2>&1; then + echo "Cross-host login wrote" >&2; exit 1 +fi +[[ ! -s "$LOG_FILE" ]] || { echo "Cross-host login reached curl" >&2; exit 1; } + +if run_wrapper -n 42 --draft --ready >/dev/null 2>&1; then echo "Accepted conflicting modes" >&2; exit 1; fi +if run_wrapper -n 42 >/dev/null 2>&1; then echo "Accepted no-op edit" >&2; exit 1; fi +run_wrapper --help 2>&1 | grep -q '^Usage:' + +# GitHub retains provider-native edit/readiness behavior. git -C "$REPO_DIR" remote set-url origin https://github.com/acme/widgets.git -: > "$LOG_FILE" -run_wrapper -n 7 --title 'GitHub title' --draft >/dev/null +: > "$LOG_FILE"; run_wrapper -n 7 --title 'GitHub title' --draft >/dev/null grep -q 'gh <pr> <edit> <7> <--title> <GitHub title>' "$LOG_FILE" grep -q 'gh <pr> <ready> <7> <--undo>' "$LOG_FILE" - echo "PR edit regression harness passed"