From 57919c38d8100878ed57778ec8853cac70db3d49 Mon Sep 17 00:00:00 2001 From: "jason.woltje" Date: Sat, 20 Jun 2026 10:16:38 +0000 Subject: [PATCH] =?UTF-8?q?fix(framework/tools):=20wrapper=20hardening=20?= =?UTF-8?q?=E2=80=94=20TLS=20validation,=20cred-path=20fallback,=20no-CI?= =?UTF-8?q?=20fast-exit=20(#551)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- .gitignore | 3 + .../559-560-wrapper-eval-login-20260620.md | 87 +++++++++++++++ .../framework/tools/_lib/credentials.sh | 29 ++++- .../framework/tools/git/detect-platform.sh | 38 +++++++ .../framework/tools/git/issue-comment.sh | 10 +- .../mosaic/framework/tools/git/pr-ci-wait.sh | 89 +++++++++++++++ .../tools/git/test-gitea-login-resolution.sh | 77 +++++++++++++ .../git/test-issue-create-body-safety.sh | 102 ++++++++++++++++++ .../mosaic/framework/tools/woodpecker/_lib.sh | 2 +- .../tools/woodpecker/pipeline-list.sh | 2 +- .../tools/woodpecker/pipeline-status.sh | 2 +- .../tools/woodpecker/pipeline-trigger.sh | 2 +- 12 files changed, 434 insertions(+), 9 deletions(-) create mode 100644 docs/scratchpads/559-560-wrapper-eval-login-20260620.md create mode 100755 packages/mosaic/framework/tools/git/test-issue-create-body-safety.sh diff --git a/.gitignore b/.gitignore index 7728aae..27556b3 100644 --- a/.gitignore +++ b/.gitignore @@ -12,3 +12,6 @@ docs/reports/ # Step-CA dev password — real file is gitignored; commit only the .example infra/step-ca/dev-password + +# Scratch dirs created by the framework git-wrapper shell test harnesses +.mosaic-test-work/ diff --git a/docs/scratchpads/559-560-wrapper-eval-login-20260620.md b/docs/scratchpads/559-560-wrapper-eval-login-20260620.md new file mode 100644 index 0000000..0fe5a7e --- /dev/null +++ b/docs/scratchpads/559-560-wrapper-eval-login-20260620.md @@ -0,0 +1,87 @@ +# Wrapper hardening fold-in: #559 (eval removal) + #560 (host-derived login) + +**Branch:** `fix/wrapper-hardening-tls-credpath-cicwait` (PR #551) +**Worker:** coderlite0 (Sonnet lane) · coordinated by mos-claude +**Date:** 2026-06-20 +**Scope:** `packages/mosaic/framework/tools/git/*.sh` only + +## What the issues asked for vs. what was already landed + +Both issues were largely satisfied by prior merged work; this fold-in closes the +remaining gaps (regression tests + a loud diagnostic + one residual word-split site) +rather than re-implementing finished functionality. + +### #559 — remove `eval` from issue-create.sh (and siblings) + +- `eval`-based command construction was already removed across the wrapper surface + (landed in #549). A full scan of `tools/git/*.sh` finds **zero** `eval` usages. +- `issue-create.sh`, `pr-create.sh`, `issue-edit.sh`, `issue-assign.sh` already build + their `tea`/`gh` invocations as argv arrays (`CMD=(...)`, `"${CMD[@]}"`), so Markdown + bodies pass through verbatim. +- **Residual found & fixed:** `issue-comment.sh` still used unquoted + `$(get_gitea_repo_args)` word-splitting (the comment body itself was already safely + quoted, so no injection bug — but it was the inconsistent, fragile pattern #559 targets, + and it failed silently when no login resolved). Converted to an argv array with an + explicit, loud login-resolution error. +- **Added regression test:** `test-issue-create-body-safety.sh` — feeds a hostile + Markdown body (`$(touch SENTINEL)`, backticks, single/double quotes, `$HOME`/`${PATH}`, + pipes/`&&`/`;`) through `issue-create.sh` and asserts (1) no command substitution + executes (sentinel file never created) and (2) the `--description` `tea` receives is + byte-for-byte the original body. + +### #560 — auto-detect Gitea `--login` from repo origin host + +- Centralized host→login resolution already exists in `detect-platform.sh` + (`get_gitea_login_for_host` → `find_tea_login_for_host`, matching `urlparse(url).hostname`). + Every wrapper routes through it (or `get_gitea_login` / `get_gitea_login_for_repo_override`); + **no wrapper hardcodes `${GITEA_LOGIN:-mosaicstack}`**. Explicit `GITEA_LOGIN` wins only + when it matches the host (`tea_login_matches_host`), so stale overrides are rejected. +- **Gap fixed — silent failure → loud diagnostic:** the failure path of + `get_gitea_login_for_host` returned non-zero with no message. Added + `print_gitea_login_diagnostic`, emitted to **stderr** on resolution failure: names the + unresolved host, lists available tea logins (name + host), and gives the `GITEA_LOGIN` + override + `tea login add` fix. Stderr-only, so it never contaminates stdout (the + resolved login name) or the log-grep assertions in the existing harnesses. Callers with + an API fallback (pr-merge, issue-close, pr-create, issue-create) still follow with their + own "using API fallback" line, giving a clear "no login → fallback" trail. +- **Extended test:** `test-gitea-login-resolution.sh` now also asserts (a) the loud + diagnostic fires and lists available logins for an unresolved host, (b) login is derived + from origin host for **both** instances (mosaicstack + usc) via a scoped second `tea` + mock, and (c) a valid `GITEA_LOGIN` override is honored. The scoped mock keeps the + existing API-fallback assertions (which require mosaicstack to have _no_ tea login) valid. + +## Files changed (wrapper surface only) + +- `detect-platform.sh` — add `print_gitea_login_diagnostic`; call it on the + `get_gitea_login_for_host` failure path. +- `issue-comment.sh` — argv array + loud login-resolution error (was unquoted + `$(get_gitea_repo_args)`). +- `test-issue-create-body-safety.sh` — **new** (#559 regression). +- `test-gitea-login-resolution.sh` — extended (#560 diagnostic + both-host + override). + +## Verification + +All wrapper harnesses pass locally: + +- `test-issue-create-body-safety.sh` — PASS +- `test-gitea-login-resolution.sh` — PASS +- `test-pr-merge-gitea-empty-uid.sh` — PASS +- `test-pr-metadata-gitea.sh` — PASS +- `test-lane-brief-pr-linkage.sh` — PASS + +## Open items flagged to mos-claude (orchestrator decisions) + +1. **CHANGELOG absent.** The task said "update CHANGELOG (append-only), keep the existing + #550/#551 entry." No CHANGELOG file exists anywhere in the repo, and #550/#551 are not + recorded in one. **ASSUMPTION:** documenting #559/#560 in this scratchpad + the PR + description (`Closes #559 Closes #560`) follows the repo's actual convention + (`docs/scratchpads/`). Did not invent a new CHANGELOG structure. +2. **`docs/TASKS.md` is orchestrator single-writer.** It carries a "Workers read but never + modify" banner. As a worker I did **not** edit it; task tracking is via the linked Gitea + issues #559/#560 + this scratchpad. Orchestrator may add a rollup row if desired. +3. **Wrapper `test-*.sh` are not CI-wired.** `.woodpecker/ci.yml` runs `pnpm +typecheck/lint/format:check/test` (`turbo run test`); the framework dir has no + `package.json`, so these shell harnesses run **locally/manually only** — they do not gate + the PR in Woodpecker. **ASSUMPTION:** out of scope to wire a shell-test step into CI in + this PR (would broaden the diff beyond the wrapper surface). Flagging for a follow-up if + the fleet wants these gated. diff --git a/packages/mosaic/framework/tools/_lib/credentials.sh b/packages/mosaic/framework/tools/_lib/credentials.sh index bbb942c..5a26689 100755 --- a/packages/mosaic/framework/tools/_lib/credentials.sh +++ b/packages/mosaic/framework/tools/_lib/credentials.sh @@ -16,7 +16,12 @@ # After loading, service-specific env vars are exported. # Run `load_credentials --help` for details. -MOSAIC_CREDENTIALS_FILE="${MOSAIC_CREDENTIALS_FILE:-$HOME/src/jarvis-brain/credentials.json}" +if [[ -z "${MOSAIC_CREDENTIALS_FILE:-}" ]]; then + for _cand in "$HOME/.config/mosaic/credentials.json" "$HOME/src/jarvis-brain/credentials.json"; do + if [[ -f "$_cand" ]]; then MOSAIC_CREDENTIALS_FILE="$_cand"; break; fi + done + : "${MOSAIC_CREDENTIALS_FILE:=$HOME/src/jarvis-brain/credentials.json}" +fi _mosaic_require_jq() { if ! command -v jq &>/dev/null; then @@ -34,6 +39,19 @@ _mosaic_read_cred() { jq -r "$jq_path // empty" "$MOSAIC_CREDENTIALS_FILE" } +# Decide curl TLS flag for a target URL: validate public hosts (MITM matters on +# WAN); allow self-signed only for private-network IP literals (trusted LAN) or an +# explicit $MOSAIC_INSECURE_TLS opt-in. Echoes "-k" or "" (empty). +_mosaic_tls_opt() { + local url="$1" host + [[ -n "${MOSAIC_INSECURE_TLS:-}" ]] && { echo "-k"; return; } + host=$(printf '%s' "$url" | sed -E 's#^[a-zA-Z]+://([^/:]+).*#\1#') + if [[ "$host" =~ ^(10\.|127\.|192\.168\.|172\.(1[6-9]|2[0-9]|3[01])\.) ]]; then + echo "-k"; return + fi + echo "" +} + # Sync Woodpecker credentials to ~/.woodpecker/.env # Only writes when values differ to avoid unnecessary disk writes. _mosaic_sync_woodpecker_env() { @@ -261,7 +279,8 @@ mosaic_http() { local base_url="${4:-}" local response - response=$(curl -sk -w "\n%{http_code}" -X "$method" \ + local _tls; _tls=$(_mosaic_tls_opt "${base_url}${endpoint}") + response=$(curl -sS $_tls -w "\n%{http_code}" -X "$method" \ -H "$auth_header" \ -H "Content-Type: application/json" \ "${base_url}${endpoint}") @@ -279,7 +298,8 @@ mosaic_http_post() { local base_url="${4:-}" local response - response=$(curl -sk -w "\n%{http_code}" -X POST \ + local _tls; _tls=$(_mosaic_tls_opt "${base_url}${endpoint}") + response=$(curl -sS $_tls -w "\n%{http_code}" -X POST \ -H "$auth_header" \ -H "Content-Type: application/json" \ -d "$data" \ @@ -297,7 +317,8 @@ mosaic_http_patch() { local base_url="${4:-}" local response - response=$(curl -sk -w "\n%{http_code}" -X PATCH \ + local _tls; _tls=$(_mosaic_tls_opt "${base_url}${endpoint}") + response=$(curl -sS $_tls -w "\n%{http_code}" -X PATCH \ -H "$auth_header" \ -H "Content-Type: application/json" \ -d "$data" \ diff --git a/packages/mosaic/framework/tools/git/detect-platform.sh b/packages/mosaic/framework/tools/git/detect-platform.sh index 58ac2fe..1b32c02 100755 --- a/packages/mosaic/framework/tools/git/detect-platform.sh +++ b/packages/mosaic/framework/tools/git/detect-platform.sh @@ -169,6 +169,43 @@ raise SystemExit(1) PY } +# Emit an actionable diagnostic to stderr when no tea login resolves for a host. +# Callers that have a working API fallback may ignore the non-zero return of +# get_gitea_login_for_host; this turns the previously SILENT failure into a loud, +# greppable hint (available logins + override + add-login instructions). Printed to +# stderr only, so it never contaminates stdout (the resolved login name) or log +# assertions that capture tea/curl invocations. +print_gitea_login_diagnostic() { + local host="${1:-}" + local available + available=$( + command -v tea >/dev/null 2>&1 || { echo "(tea CLI not installed)"; exit 0; } + logins_json=$(tea login list --output json 2>/dev/null) || { echo "(could not query tea login list)"; exit 0; } + TEA_LOGINS_JSON="$logins_json" python3 - <<'PY' +import json, os +from urllib.parse import urlparse +try: + logins = json.loads(os.environ.get("TEA_LOGINS_JSON", "[]")) +except Exception: + logins = [] +rows = [] +for login in logins if isinstance(logins, list) else []: + name = str(login.get("name") or login.get("Name") or "") + url = str(login.get("url") or login.get("URL") or "") + host = urlparse(url).hostname or "?" + if name: + rows.append(f"{name} (host: {host})") +print("; ".join(rows) if rows else "(none configured)") +PY + ) + { + echo "Error: no Gitea tea login matches host '$host'." + echo " Available tea logins: ${available}" + echo " Fix: set GITEA_LOGIN to a login whose URL host is '$host'," + echo " or add one: tea login add --name --url https://$host --token " + } >&2 +} + get_gitea_login_for_host() { local host="${1:-}" local login @@ -190,6 +227,7 @@ get_gitea_login_for_host() { return 0 fi + print_gitea_login_diagnostic "$host" return 1 } diff --git a/packages/mosaic/framework/tools/git/issue-comment.sh b/packages/mosaic/framework/tools/git/issue-comment.sh index 70f4a42..5fef417 100755 --- a/packages/mosaic/framework/tools/git/issue-comment.sh +++ b/packages/mosaic/framework/tools/git/issue-comment.sh @@ -53,7 +53,15 @@ if [[ "$PLATFORM" == "github" ]]; then gh issue comment "$ISSUE_NUMBER" --body "$COMMENT" echo "Added comment to GitHub issue #$ISSUE_NUMBER" elif [[ "$PLATFORM" == "gitea" ]]; then - tea issue comment "$ISSUE_NUMBER" "$COMMENT" $(get_gitea_repo_args) + # Build the invocation as an argv array (not unquoted $(get_gitea_repo_args) + # word-splitting) so the comment body — including Markdown backticks, $(...), + # and quotes — is passed verbatim and never re-split or shell-evaluated. + REPO_SLUG=$(get_repo_slug) + GITEA_LOGIN_NAME=$(get_gitea_login) || { + echo "Error: could not resolve a Gitea login for this repo; cannot comment on issue #$ISSUE_NUMBER." >&2 + exit 1 + } + tea issue comment "$ISSUE_NUMBER" "$COMMENT" --repo "$REPO_SLUG" --login "$GITEA_LOGIN_NAME" echo "Added comment to Gitea issue #$ISSUE_NUMBER" else echo "Error: Unknown platform" diff --git a/packages/mosaic/framework/tools/git/pr-ci-wait.sh b/packages/mosaic/framework/tools/git/pr-ci-wait.sh index 7b9e8b6..1a47759 100755 --- a/packages/mosaic/framework/tools/git/pr-ci-wait.sh +++ b/packages/mosaic/framework/tools/git/pr-ci-wait.sh @@ -72,6 +72,11 @@ elif values and all(v == "success" for v in values): print("success") elif any(v in {"pending", "running", "queued", "waiting"} for v in values): print("pending") +elif not values and not state: + # No pipeline/status of any kind reported for this commit. Distinct from + # "unknown" (an ambiguous/unrecognized status that should keep polling): + # this signals a repo/commit that simply has no CI configured. + print("no-status") else: print("unknown") PY @@ -142,6 +147,21 @@ gitea_get_commit_status_json() { curl -fsSL -H "User-Agent: curl/8" -H "Authorization: token ${token}" "$url" } +gitea_get_default_branch() { + local host="$1" + local repo="$2" + local token="$3" + local url="https://${host}/api/v1/repos/${repo}" + curl -fsSL -H "User-Agent: curl/8" -H "Authorization: token ${token}" "$url" | python3 -c ' +import json, sys +print((json.load(sys.stdin) or {}).get("default_branch", "")) +' +} + +github_get_default_branch() { + gh api "repos/${OWNER}/${REPO}" --jq '.default_branch' +} + while [[ $# -gt 0 ]]; do case "$1" in -n|--number) @@ -245,6 +265,51 @@ else exit 1 fi +# No-CI determination is TWO-TIER (primary: CI history; secondary: empty-poll streak). +# +# PRIMARY — "does this repo run CI at all?" Probed once, up front, from the DEFAULT +# BRANCH's commit status. A repo whose default branch carries CI statuses +# demonstrably runs CI, so an EMPTY status on the PR head means the pipeline simply +# has not registered YET (webhook/queue lag) — NOT that the repo is CI-less. In that +# case we must NEVER fast-green; we keep polling until the pipeline registers or the +# timeout fires (both safe). This closes the webhook-lag false-green: a slow-to- +# register pipeline feeding a merge gate can no longer be mistaken for "no CI". +# +# SECONDARY — the empty-poll streak below applies ONLY to genuinely CI-less repos +# (default branch also has no CI history, e.g. device-imaging class), where burning +# the full timeout would be pure waste. There, NO_CI_MAX empty polls => fast-exit 0. +# +# Probe failure is treated conservatively as REPO_HAS_CI=1 (assume CI present): we +# would rather wait-then-timeout than risk a false-green, per the merge-gate priority. +REPO_HAS_CI=1 +detect_repo_ci() { + local def_branch def_status + # Every early exit returns 0: a probe miss must leave the conservative + # REPO_HAS_CI=1 default in place, never abort the caller under `set -e`. + if [[ "$PLATFORM" == "github" ]]; then + def_branch=$(github_get_default_branch 2>/dev/null) || { + echo "[pr-ci-wait] WARN: default-branch probe failed; assuming CI-enabled (will not fast-green on empty status)."; return 0; } + [[ -n "$def_branch" ]] || return 0 + def_status=$(github_get_commit_status_json "$OWNER" "$REPO" "$def_branch" 2>/dev/null | extract_state_from_status_json) || return 0 + else + def_branch=$(gitea_get_default_branch "$HOST" "$OWNER/$REPO" "$TOKEN" 2>/dev/null) || { + echo "[pr-ci-wait] WARN: default-branch probe failed; assuming CI-enabled (will not fast-green on empty status)."; return 0; } + [[ -n "$def_branch" ]] || return 0 + def_status=$(gitea_get_commit_status_json "$HOST" "$OWNER/$REPO" "$TOKEN" "$def_branch" 2>/dev/null | extract_state_from_status_json) || return 0 + fi + if [[ "$def_status" == "no-status" || -z "$def_status" ]]; then + REPO_HAS_CI=0 + echo "[pr-ci-wait] default branch '${def_branch}' has no CI status history — treating repo as CI-less (empty-poll fast-exit enabled)." + else + REPO_HAS_CI=1 + echo "[pr-ci-wait] default branch '${def_branch}' has CI history (state=${def_status}) — repo runs CI; empty status on PR head => awaiting registration, will not fast-green." + fi +} +detect_repo_ci || true + +NO_CI_STREAK=0 +NO_CI_MAX=3 + while true; do NOW_TS=$(date +%s) if (( NOW_TS > DEADLINE_TS )); then @@ -272,11 +337,35 @@ while true; do echo "Error: CI reported ${STATE} for PR #$PR_NUMBER." >&2 exit 1 ;; + no-status) + if [[ "$REPO_HAS_CI" == "1" ]]; then + # PRIMARY tier: repo demonstrably runs CI but this commit's pipeline + # has not registered yet (webhook/queue lag). Do NOT fast-green — keep + # polling until it registers or the timeout fires. Reset the streak so + # a later genuine CI-less misread can't accumulate across this state. + NO_CI_STREAK=0 + echo "[pr-ci-wait] empty status on PR head but repo runs CI — awaiting pipeline registration (webhook lag), not fast-greening." + else + # SECONDARY tier: genuinely CI-less repo (default branch has no CI + # history either). Empty polls => fast-exit green after NO_CI_MAX. + NO_CI_STREAK=$((NO_CI_STREAK + 1)) + if (( NO_CI_STREAK >= NO_CI_MAX )); then + echo "[INFO] no CI configured for this repo/commit (PR #$PR_NUMBER, ${NO_CI_STREAK} consecutive empty polls, default branch also CI-less); treating as green." + exit 0 + fi + fi + sleep "$INTERVAL_SEC" + ;; pending|unknown) + # A pipeline exists but hasn't reached a terminal state (or is + # transiently ambiguous) — keep waiting, and reset the no-CI streak + # since this commit is not in the "no CI at all" condition. + NO_CI_STREAK=0 sleep "$INTERVAL_SEC" ;; *) echo "[pr-ci-wait] Unrecognized state '${STATE}', continuing to poll..." + NO_CI_STREAK=0 sleep "$INTERVAL_SEC" ;; esac diff --git a/packages/mosaic/framework/tools/git/test-gitea-login-resolution.sh b/packages/mosaic/framework/tools/git/test-gitea-login-resolution.sh index 411848f..3a875df 100755 --- a/packages/mosaic/framework/tools/git/test-gitea-login-resolution.sh +++ b/packages/mosaic/framework/tools/git/test-gitea-login-resolution.sh @@ -230,4 +230,81 @@ if grep -q -- 'tea issue close 536 .*--login mosaicstack' "$LOG_FILE"; then exit 1 fi +# --------------------------------------------------------------------------- +# #560: loud diagnostic + host-derived login for BOTH instances + override-wins +# --------------------------------------------------------------------------- + +# Loud diagnostic: a host with no matching tea login must emit an actionable +# error to stderr (the previous behavior was a SILENT failure). The original +# mock defines only usc/evil-usc logins, so mosaicstack resolution fails here. +git -C "$REPO_DIR" remote set-url origin https://git.mosaicstack.dev/mosaicstack/stack.git +diag_stderr=$(run_in_repo bash -c ' + source "'"$SCRIPT_DIR"'/detect-platform.sh" + get_gitea_login_for_host git.mosaicstack.dev +' 2>&1 1>/dev/null || true) +if ! grep -q "no Gitea tea login matches host 'git.mosaicstack.dev'" <<<"$diag_stderr"; then + echo "Expected loud diagnostic naming the unresolved host; got: $diag_stderr" >&2 + exit 1 +fi +if ! grep -q "Available tea logins:" <<<"$diag_stderr"; then + echo "Expected diagnostic to list available tea logins; got: $diag_stderr" >&2 + exit 1 +fi + +# Both-instance host derivation + override-wins, using a mock that DOES define a +# mosaicstack login. Scoped to this section so the API-fallback assertions above +# (which rely on mosaicstack having NO tea login) remain valid. +BIN_DIR2="$WORK_DIR/bin2" +mkdir -p "$BIN_DIR2" +cp "$BIN_DIR/curl" "$BIN_DIR2/curl" +cat > "$BIN_DIR2/tea" <<'SH' +#!/usr/bin/env bash +set -euo pipefail +if [[ "$*" == "login list --output json" ]]; then + cat <<'JSON' +[ + {"name":"mosaicstack","url":"https://git.mosaicstack.dev","user":"jason.woltje"}, + {"name":"usc","url":"https://git.uscllc.com","user":"jason.woltje"} +] +JSON + exit 0 +fi +printf 'tea %s\n' "$*" >> "$MOSAIC_TEST_LOG" +exit 0 +SH +chmod +x "$BIN_DIR2/tea" + +run_in_repo2() { + ( + cd "$REPO_DIR" + PATH="$BIN_DIR2:$PATH" \ + MOSAIC_CREDENTIALS_FILE="$CREDENTIALS_FILE" \ + MOSAIC_TEST_LOG="$LOG_FILE" \ + "$@" + ) +} + +git -C "$REPO_DIR" remote set-url origin https://git.mosaicstack.dev/mosaicstack/stack.git +mosaic_login=$(run_in_repo2 bash -c 'source "'"$SCRIPT_DIR"'/detect-platform.sh"; get_gitea_login') +if [[ "$mosaic_login" != "mosaicstack" ]]; then + echo "Expected mosaicstack origin to derive login 'mosaicstack'; got '$mosaic_login'" >&2 + exit 1 +fi + +git -C "$REPO_DIR" remote set-url origin https://git.uscllc.com/USC/uconnect.git +usc_login_derived=$(run_in_repo2 bash -c 'source "'"$SCRIPT_DIR"'/detect-platform.sh"; get_gitea_login') +if [[ "$usc_login_derived" != "usc" ]]; then + echo "Expected usc origin to derive login 'usc'; got '$usc_login_derived'" >&2 + exit 1 +fi + +# Explicit GITEA_LOGIN override is honored when it matches the host. +git -C "$REPO_DIR" remote set-url origin https://git.mosaicstack.dev/mosaicstack/stack.git +override_wins=$(run_in_repo2 bash -c 'export GITEA_LOGIN=mosaicstack; source "'"$SCRIPT_DIR"'/detect-platform.sh"; get_gitea_login') +if [[ "$override_wins" != "mosaicstack" ]]; then + echo "Expected valid GITEA_LOGIN override to win on mosaicstack host; got '$override_wins'" >&2 + exit 1 +fi +git -C "$REPO_DIR" remote set-url origin https://git.uscllc.com/USC/uconnect.git + echo "Gitea login resolution regression harness passed" diff --git a/packages/mosaic/framework/tools/git/test-issue-create-body-safety.sh b/packages/mosaic/framework/tools/git/test-issue-create-body-safety.sh new file mode 100755 index 0000000..e25a012 --- /dev/null +++ b/packages/mosaic/framework/tools/git/test-issue-create-body-safety.sh @@ -0,0 +1,102 @@ +#!/usr/bin/env bash +# Regression harness for issue-create.sh Markdown-body safety (#559). +# +# Guards against reintroduction of eval-based command construction. The wrapper +# builds its tea/gh invocation as an argv array, so a body containing command +# substitution ($(...)), backticks, quotes, and dollar signs MUST reach tea +# verbatim and MUST NOT be shell-evaluated. This test asserts both: +# 1. No command-substitution side effect (an injected `touch SENTINEL` never runs). +# 2. The --description value tea receives is byte-for-byte the original body. + +set -euo pipefail + +SCRIPT_DIR="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)" +WORK_DIR="${MOSAIC_TEST_WORK_DIR:-$PWD/.mosaic-test-work/issue-create-body-safety}" +REPO_DIR="$WORK_DIR/repo" +BIN_DIR="$WORK_DIR/bin" +SENTINEL="$WORK_DIR/INJECTION_SENTINEL" +BODY_FILE="$WORK_DIR/body.txt" +RECEIVED_FILE="$WORK_DIR/received-description.txt" + +rm -rf "$WORK_DIR" +mkdir -p "$REPO_DIR" "$BIN_DIR" + +git -C "$REPO_DIR" init -q +git -C "$REPO_DIR" remote add origin https://git.mosaicstack.dev/mosaicstack/stack.git + +# Hostile Markdown body. The unquoted heredoc expands $SENTINEL (a real path we +# want embedded) but every shell metacharacter we care about is backslash-escaped +# so the TEST shell writes them literally into the file — the bytes the wrapper +# must then preserve. +cat > "$BODY_FILE" < "$BIN_DIR/tea" <<'SH' +#!/usr/bin/env bash +set -euo pipefail + +if [[ "$*" == "login list --output json" ]]; then + cat <<'JSON' +[ + {"name":"mosaicstack","url":"https://git.mosaicstack.dev","user":"jason.woltje"} +] +JSON + exit 0 +fi + +if [[ "${1:-}" == "issue" && "${2:-}" == "create" ]]; then + desc="" + while [[ $# -gt 0 ]]; do + case "$1" in + --description) desc="$2"; shift 2 ;; + *) shift ;; + esac + done + printf '%s' "$desc" > "$MOSAIC_TEST_RECEIVED" + echo "#1 created" + exit 0 +fi + +exit 0 +SH +chmod +x "$BIN_DIR/tea" + +( + cd "$REPO_DIR" + PATH="$BIN_DIR:$PATH" \ + MOSAIC_TEST_RECEIVED="$RECEIVED_FILE" \ + "$SCRIPT_DIR/issue-create.sh" -t "Body safety test" -b "$BODY" +) >/dev/null + +# 1. No command substitution executed anywhere in the pipeline. +if [[ -e "$SENTINEL" ]]; then + echo "FAIL: injected command substitution executed (sentinel file created): $SENTINEL" >&2 + exit 1 +fi + +# 2. tea actually received the body (issue create path taken, not silently dropped). +if [[ ! -f "$RECEIVED_FILE" ]]; then + echo "FAIL: tea issue create was never invoked with a --description" >&2 + exit 1 +fi + +# 3. The description tea received is byte-for-byte the original body. +if [[ "$(cat "$RECEIVED_FILE")" != "$BODY" ]]; then + echo "FAIL: body was not preserved verbatim through issue-create.sh" >&2 + echo "--- expected ---" >&2; printf '%s\n' "$BODY" >&2 + echo "--- received ---" >&2; cat "$RECEIVED_FILE" >&2 + exit 1 +fi + +echo "issue-create.sh Markdown body-safety regression harness passed" diff --git a/packages/mosaic/framework/tools/woodpecker/_lib.sh b/packages/mosaic/framework/tools/woodpecker/_lib.sh index bb9c0e6..899fa1a 100755 --- a/packages/mosaic/framework/tools/woodpecker/_lib.sh +++ b/packages/mosaic/framework/tools/woodpecker/_lib.sh @@ -12,7 +12,7 @@ wp_resolve_repo_id() { local full_name="$1" local response http_code body repo_id - response=$(curl -sk -w "\n%{http_code}" \ + response=$(curl -sS -w "\n%{http_code}" \ -H "Authorization: Bearer $WOODPECKER_TOKEN" \ "${WOODPECKER_URL}/api/repos/lookup/${full_name}") diff --git a/packages/mosaic/framework/tools/woodpecker/pipeline-list.sh b/packages/mosaic/framework/tools/woodpecker/pipeline-list.sh index d77ca37..60d9aa2 100755 --- a/packages/mosaic/framework/tools/woodpecker/pipeline-list.sh +++ b/packages/mosaic/framework/tools/woodpecker/pipeline-list.sh @@ -48,7 +48,7 @@ fi # Resolve owner/repo to numeric ID (Woodpecker v3 API) REPO_ID=$(wp_resolve_repo_id "$REPO") || exit 1 -response=$(curl -sk -w "\n%{http_code}" \ +response=$(curl -sS -w "\n%{http_code}" \ -H "Authorization: Bearer $WOODPECKER_TOKEN" \ "${WOODPECKER_URL}/api/repos/${REPO_ID}/pipelines?perPage=${LIMIT}") diff --git a/packages/mosaic/framework/tools/woodpecker/pipeline-status.sh b/packages/mosaic/framework/tools/woodpecker/pipeline-status.sh index 6fbd186..0996eb8 100755 --- a/packages/mosaic/framework/tools/woodpecker/pipeline-status.sh +++ b/packages/mosaic/framework/tools/woodpecker/pipeline-status.sh @@ -50,7 +50,7 @@ REPO_ID=$(wp_resolve_repo_id "$REPO") || exit 1 _wp_fetch() { local ep="$1" local resp http_code body - resp=$(curl -sk -w "\n%{http_code}" \ + resp=$(curl -sS -w "\n%{http_code}" \ -H "Authorization: Bearer $WOODPECKER_TOKEN" \ "$ep") http_code=$(echo "$resp" | tail -n1) diff --git a/packages/mosaic/framework/tools/woodpecker/pipeline-trigger.sh b/packages/mosaic/framework/tools/woodpecker/pipeline-trigger.sh index d8e497a..5d31fd2 100755 --- a/packages/mosaic/framework/tools/woodpecker/pipeline-trigger.sh +++ b/packages/mosaic/framework/tools/woodpecker/pipeline-trigger.sh @@ -46,7 +46,7 @@ REPO_ID=$(wp_resolve_repo_id "$REPO") || exit 1 echo "Triggering pipeline for $REPO on branch $BRANCH..." -response=$(curl -sk -w "\n%{http_code}" -X POST \ +response=$(curl -sS -w "\n%{http_code}" -X POST \ -H "Authorization: Bearer $WOODPECKER_TOKEN" \ -H "Content-Type: application/json" \ -d "$(jq -n --arg b "$BRANCH" '{branch: $b}')" \