Compare commits
5 Commits
feat/pi-mo
...
feb0d8a58b
| Author | SHA1 | Date | |
|---|---|---|---|
|
|
feb0d8a58b | ||
|
|
9b7e63f6c3 | ||
|
|
b23a7e81ae | ||
| 87f561c1f8 | |||
| 8c45857859 |
3
.gitignore
vendored
3
.gitignore
vendored
@@ -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/
|
||||
|
||||
87
docs/scratchpads/559-560-wrapper-eval-login-20260620.md
Normal file
87
docs/scratchpads/559-560-wrapper-eval-login-20260620.md
Normal file
@@ -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.
|
||||
@@ -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/<instance>.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" \
|
||||
|
||||
@@ -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:-<unknown>}"
|
||||
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 <name> --url https://$host --token <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
|
||||
}
|
||||
|
||||
|
||||
@@ -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"
|
||||
|
||||
@@ -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
|
||||
|
||||
@@ -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"
|
||||
|
||||
102
packages/mosaic/framework/tools/git/test-issue-create-body-safety.sh
Executable file
102
packages/mosaic/framework/tools/git/test-issue-create-body-safety.sh
Executable file
@@ -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" <<EOF
|
||||
# Release notes
|
||||
|
||||
Inline code: \`rm -rf /\` must stay literal.
|
||||
Command sub attempt: \$(touch $SENTINEL)
|
||||
Backtick cmd attempt: \`touch $SENTINEL\`
|
||||
Dollars: \$HOME \${PATH} \$5.00 and 100% done
|
||||
Quotes: "double" and 'single' and \`mixed\`
|
||||
Trailing pipe-ish: foo | bar && baz ; qux
|
||||
EOF
|
||||
|
||||
BODY="$(cat "$BODY_FILE")"
|
||||
|
||||
# Mock tea: resolve a mosaicstack login, then capture the --description verbatim.
|
||||
cat > "$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"
|
||||
@@ -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}")
|
||||
|
||||
|
||||
@@ -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}")
|
||||
|
||||
|
||||
@@ -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)
|
||||
|
||||
@@ -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}')" \
|
||||
|
||||
@@ -1,7 +1,11 @@
|
||||
import { describe, it, expect, vi, beforeEach, afterEach, type MockInstance } from 'vitest';
|
||||
import { Command } from 'commander';
|
||||
import { mkdtempSync, mkdirSync, writeFileSync, symlinkSync, rmSync } from 'node:fs';
|
||||
import { tmpdir } from 'node:os';
|
||||
import { join } from 'node:path';
|
||||
import {
|
||||
buildPiSkillArgs,
|
||||
enumerateSkillDirs,
|
||||
piForceSkillNames,
|
||||
registerRuntimeLaunchers,
|
||||
type RuntimeLaunchHandler,
|
||||
@@ -116,11 +120,101 @@ describe('buildPiSkillArgs', () => {
|
||||
]);
|
||||
});
|
||||
|
||||
it('force-loads fleet skills even under native Pi discovery', () => {
|
||||
it('force-loads fleet skills under native Pi discovery when not already discoverable', () => {
|
||||
// Empty native set => Pi would not find mosaic-tools on its own, so force it.
|
||||
expect(
|
||||
buildPiSkillArgs([], { MOSAIC_PI_SKILL_MODE: 'discover' }, fakeSkills, fakeForced),
|
||||
buildPiSkillArgs([], { MOSAIC_PI_SKILL_MODE: 'discover' }, fakeSkills, fakeForced, new Set()),
|
||||
).toEqual(['--skill', '/skills/mosaic-tools']);
|
||||
});
|
||||
|
||||
it('discover mode drops a forced skill Pi already discovers natively (no double-load)', () => {
|
||||
// mosaic-tools is reachable from a Pi native root, so native discovery
|
||||
// covers it — forcing it again would register the same skill twice.
|
||||
expect(
|
||||
buildPiSkillArgs(
|
||||
[],
|
||||
{ MOSAIC_PI_SKILL_MODE: 'discover' },
|
||||
fakeSkills,
|
||||
fakeForced,
|
||||
new Set(['/skills/mosaic-tools']),
|
||||
),
|
||||
).toEqual([]);
|
||||
});
|
||||
|
||||
it('discover mode keeps a forced skill that no native root provides', () => {
|
||||
expect(
|
||||
buildPiSkillArgs(
|
||||
[],
|
||||
{ MOSAIC_PI_SKILL_MODE: 'discover' },
|
||||
fakeSkills,
|
||||
fakeForced,
|
||||
new Set(['/skills/some-other-skill']),
|
||||
),
|
||||
).toEqual(['--skill', '/skills/mosaic-tools']);
|
||||
});
|
||||
|
||||
it('discover mode collapses a forced skill listed twice to a single --skill', () => {
|
||||
// Mirror 'all' mode: intra-forced-set duplicates (same realpath) dedup.
|
||||
expect(
|
||||
buildPiSkillArgs(
|
||||
[],
|
||||
{ MOSAIC_PI_SKILL_MODE: 'discover' },
|
||||
fakeSkills,
|
||||
['--skill', '/skills/mosaic-tools', '--skill', '/skills/mosaic-tools'],
|
||||
new Set(),
|
||||
),
|
||||
).toEqual(['--skill', '/skills/mosaic-tools']);
|
||||
});
|
||||
});
|
||||
|
||||
describe('enumerateSkillDirs (real FS)', () => {
|
||||
let root: string;
|
||||
|
||||
beforeEach(() => {
|
||||
root = mkdtempSync(join(tmpdir(), 'mosaic-skills-'));
|
||||
});
|
||||
|
||||
afterEach(() => {
|
||||
rmSync(root, { recursive: true, force: true });
|
||||
});
|
||||
|
||||
function makeSkill(parent: string, name: string): string {
|
||||
const dir = join(parent, name);
|
||||
mkdirSync(dir, { recursive: true });
|
||||
writeFileSync(join(dir, 'SKILL.md'), `# ${name}\n`);
|
||||
return dir;
|
||||
}
|
||||
|
||||
it('accepts a symlinked skill dir (regression: synced fleet skills are symlinks)', () => {
|
||||
// Real skill lives under `canonical/`; the scanned root only has a symlink to it.
|
||||
const canonical = makeSkill(join(root, 'canonical'), 'mosaic-tools');
|
||||
const scanned = join(root, 'scanned');
|
||||
mkdirSync(scanned, { recursive: true });
|
||||
symlinkSync(canonical, join(scanned, 'mosaic-tools'), 'dir');
|
||||
|
||||
expect(enumerateSkillDirs([scanned])).toEqual(['--skill', join(scanned, 'mosaic-tools')]);
|
||||
});
|
||||
|
||||
it('dedups by real path when the same skill is reachable from two roots', () => {
|
||||
// Root A holds the real dir; root B symlinks to it — one --skill, not two.
|
||||
const rootA = join(root, 'a');
|
||||
const rootB = join(root, 'b');
|
||||
const real = makeSkill(rootA, 'mosaic-tools');
|
||||
mkdirSync(rootB, { recursive: true });
|
||||
symlinkSync(real, join(rootB, 'mosaic-tools'), 'dir');
|
||||
|
||||
expect(enumerateSkillDirs([rootA, rootB])).toEqual(['--skill', real]);
|
||||
});
|
||||
|
||||
it('skips directories without a SKILL.md and missing roots', () => {
|
||||
mkdirSync(join(root, 'present', 'not-a-skill'), { recursive: true });
|
||||
makeSkill(join(root, 'present'), 'real-skill');
|
||||
|
||||
expect(enumerateSkillDirs([join(root, 'present'), join(root, 'does-not-exist')])).toEqual([
|
||||
'--skill',
|
||||
join(root, 'present', 'real-skill'),
|
||||
]);
|
||||
});
|
||||
});
|
||||
|
||||
describe('piForceSkillNames', () => {
|
||||
|
||||
@@ -6,7 +6,15 @@
|
||||
*/
|
||||
|
||||
import { execFileSync, execSync, spawnSync } from 'node:child_process';
|
||||
import { existsSync, mkdirSync, readFileSync, writeFileSync, readdirSync, rmSync } from 'node:fs';
|
||||
import {
|
||||
existsSync,
|
||||
mkdirSync,
|
||||
readFileSync,
|
||||
writeFileSync,
|
||||
readdirSync,
|
||||
realpathSync,
|
||||
rmSync,
|
||||
} from 'node:fs';
|
||||
import { createRequire } from 'node:module';
|
||||
import { homedir } from 'node:os';
|
||||
import { join, dirname } from 'node:path';
|
||||
@@ -428,25 +436,74 @@ function ensureRuntimeConfig(runtime: RuntimeName, destPath: string): void {
|
||||
|
||||
// ─── Pi skill/extension discovery ────────────────────────────────────────────
|
||||
|
||||
function discoverPiSkills(): string[] {
|
||||
/** Resolve a skill dir to its canonical real path so symlinked duplicates
|
||||
* (e.g. ~/.pi/agent/skills/X -> ~/.config/mosaic/skills/X) collapse to one key.
|
||||
* Falls back to the literal path if it can't be resolved (e.g. broken link). */
|
||||
function skillRealPath(dir: string): string {
|
||||
try {
|
||||
return realpathSync(dir);
|
||||
} catch {
|
||||
return dir;
|
||||
}
|
||||
}
|
||||
|
||||
/** Skill roots Pi auto-discovers natively (no `--skill` needed): its global
|
||||
* skills dir and the project-local one relative to the launch cwd. */
|
||||
function piNativeSkillRoots(cwd: string = process.cwd()): string[] {
|
||||
return [join(homedir(), '.pi', 'agent', 'skills'), join(cwd, '.pi', 'skills')];
|
||||
}
|
||||
|
||||
/** Enumerate skill dirs under a set of roots, deduped by real path. A directory
|
||||
* counts as a skill when it (or its symlink target) contains a SKILL.md.
|
||||
* Exported for tests (real-FS coverage of symlink acceptance + realpath dedup). */
|
||||
export function enumerateSkillDirs(roots: string[]): string[] {
|
||||
const seen = new Set<string>();
|
||||
const args: string[] = [];
|
||||
for (const skillsRoot of [join(MOSAIC_HOME, 'skills'), join(MOSAIC_HOME, 'skills-local')]) {
|
||||
for (const skillsRoot of roots) {
|
||||
if (!existsSync(skillsRoot)) continue;
|
||||
try {
|
||||
for (const entry of readdirSync(skillsRoot, { withFileTypes: true })) {
|
||||
if (!entry.isDirectory()) continue;
|
||||
// Synced fleet skills land as symlinks, so accept both dirs and links.
|
||||
if (!entry.isDirectory() && !entry.isSymbolicLink()) continue;
|
||||
const skillDir = join(skillsRoot, entry.name);
|
||||
if (existsSync(join(skillDir, 'SKILL.md'))) {
|
||||
args.push('--skill', skillDir);
|
||||
}
|
||||
if (!existsSync(join(skillDir, 'SKILL.md'))) continue;
|
||||
const key = skillRealPath(skillDir);
|
||||
if (seen.has(key)) continue;
|
||||
seen.add(key);
|
||||
args.push('--skill', skillDir);
|
||||
}
|
||||
} catch {
|
||||
// skip
|
||||
// skip unreadable roots
|
||||
}
|
||||
}
|
||||
return args;
|
||||
}
|
||||
|
||||
/** Every skill dir Pi would link under `MOSAIC_PI_SKILL_MODE=all`: the Mosaic
|
||||
* global/local catalog plus Pi's own native roots. `--no-skills` suppresses
|
||||
* native auto-discovery, so 'all' must re-add the native roots explicitly or
|
||||
* they would be silently dropped. Deduped by real path. */
|
||||
function discoverPiSkills(cwd: string = process.cwd()): string[] {
|
||||
return enumerateSkillDirs([
|
||||
join(MOSAIC_HOME, 'skills'),
|
||||
join(MOSAIC_HOME, 'skills-local'),
|
||||
...piNativeSkillRoots(cwd),
|
||||
]);
|
||||
}
|
||||
|
||||
/** Real paths of skills Pi will auto-discover from its native roots. Used to
|
||||
* drop redundant force-loads in 'discover' mode (which keeps native discovery
|
||||
* on) so the same skill is not registered twice. */
|
||||
function piNativeSkillRealPaths(cwd: string = process.cwd()): Set<string> {
|
||||
const args = enumerateSkillDirs(piNativeSkillRoots(cwd));
|
||||
const set = new Set<string>();
|
||||
for (let i = 1; i < args.length; i += 2) {
|
||||
const dir = args[i];
|
||||
if (dir !== undefined) set.add(skillRealPath(dir));
|
||||
}
|
||||
return set;
|
||||
}
|
||||
|
||||
type PiSkillMode = 'none' | 'all' | 'discover';
|
||||
|
||||
function normalizePiSkillMode(env: NodeJS.ProcessEnv): PiSkillMode {
|
||||
@@ -492,15 +549,19 @@ function forcedPiSkillArgs(env: NodeJS.ProcessEnv = process.env): string[] {
|
||||
return args;
|
||||
}
|
||||
|
||||
/** Concatenate `--skill <dir>` arg groups, dropping any directory already seen. */
|
||||
/** Concatenate `--skill <dir>` arg groups, dropping any skill already seen.
|
||||
* Dedup is by real path, so a forced skill and the same skill reached via a
|
||||
* different (e.g. symlinked) directory collapse to a single `--skill`. */
|
||||
function mergeSkillArgs(...groups: string[][]): string[] {
|
||||
const seen = new Set<string>();
|
||||
const out: string[] = [];
|
||||
for (const group of groups) {
|
||||
for (let i = 0; i < group.length; i += 2) {
|
||||
const dir = group[i + 1];
|
||||
if (group[i] !== '--skill' || dir === undefined || seen.has(dir)) continue;
|
||||
seen.add(dir);
|
||||
if (group[i] !== '--skill' || dir === undefined) continue;
|
||||
const key = skillRealPath(dir);
|
||||
if (seen.has(key)) continue;
|
||||
seen.add(key);
|
||||
out.push('--skill', dir);
|
||||
}
|
||||
}
|
||||
@@ -512,17 +573,31 @@ export function buildPiSkillArgs(
|
||||
env: NodeJS.ProcessEnv = process.env,
|
||||
discoveredSkillArgs: string[] = discoverPiSkills(),
|
||||
forcedSkillArgs: string[] = forcedPiSkillArgs(env),
|
||||
nativeSkillRealPaths: Set<string> = piNativeSkillRealPaths(),
|
||||
): string[] {
|
||||
const mode = normalizePiSkillMode(env);
|
||||
|
||||
if (mode === 'discover') {
|
||||
// Native Pi discovery handles the rest; still force-load the fleet skills.
|
||||
return [...forcedSkillArgs];
|
||||
// Native Pi discovery stays on, so only force-load fleet skills it will NOT
|
||||
// already find under its native roots — otherwise the same skill is
|
||||
// registered twice (once natively, once via --skill). mergeSkillArgs first
|
||||
// collapses any intra-forced-set realpath duplicates, mirroring 'all' mode.
|
||||
const deduped = mergeSkillArgs(forcedSkillArgs);
|
||||
const out: string[] = [];
|
||||
for (let i = 0; i < deduped.length; i += 2) {
|
||||
const dir = deduped[i + 1];
|
||||
if (deduped[i] !== '--skill' || dir === undefined) continue;
|
||||
if (nativeSkillRealPaths.has(skillRealPath(dir))) continue;
|
||||
out.push('--skill', dir);
|
||||
}
|
||||
return out;
|
||||
}
|
||||
|
||||
if (mode === 'all') {
|
||||
// 'all' links the full catalog; merge in the forced set so fleet-critical
|
||||
// skills are guaranteed present even if they live only under skills-local/.
|
||||
// discoverPiSkills already covers Pi's native roots, which `--no-skills`
|
||||
// would otherwise suppress.
|
||||
return ['--no-skills', ...mergeSkillArgs(discoveredSkillArgs, forcedSkillArgs)];
|
||||
}
|
||||
|
||||
|
||||
Reference in New Issue
Block a user