diff --git a/.github/workflows/lint.yml b/.github/workflows/lint.yml index 9c7b60ac4..6c2a2224f 100644 --- a/.github/workflows/lint.yml +++ b/.github/workflows/lint.yml @@ -118,6 +118,7 @@ jobs: tests/test_readme_refresh_helpers.bats \ tests/test_readme_refresh_push.bats \ tests/test_validate_personas.bats \ + tests/verify_persona_teams.bats \ tests/test_dev_lead_concurrency_guard.bats \ tests/test_validate_workflow_schedules.bats \ tests/caller_stub_freeze.bats \ @@ -309,6 +310,32 @@ jobs: PERSONA_SCHEMA_REF: ${{ github.head_ref || github.ref_name }} run: python3 personas/validate-personas.py personas + verify-persona-teams: + # LIVE persona addressing-team check (#1280). The hermetic validate-personas.py + # (and its hermetic bats suite) verifies the handle SHAPE — the team slug == + # id — but deliberately cannot reach the network. This separate job is the + # network-touching half: for every personas//persona.yml that declares an + # `address` block, it verifies the LIVE team named by address.handle EXISTS, is + # `privacy: closed` (secret teams are unmentionable), and is + # `notification_setting: notifications_disabled` (the handle routes a webhook, + # it must not page the team's members). Reading org teams needs an ORG-scoped + # token (GH_PAT_WORKFLOWS) — github.token cannot read teams. It FAILS CLOSED: an + # API error is never read as "the team is fine" (#755). Personas with no + # `address` block pass. It must not be folded back into validate-personas.py, + # whose hermeticity is the whole reason this was split out. + runs-on: ubuntu-latest + timeout-minutes: 5 + steps: + - uses: actions/checkout@df4cb1c069e1874edd31b4311f1884172cec0e10 # v6.0.3 + + - name: Install PyYAML + run: pip3 install --quiet pyyaml + + - name: Verify live persona addressing teams + env: + GH_TOKEN: ${{ secrets.GH_PAT_WORKFLOWS }} + run: bash scripts/verify-persona-teams.sh personas + gh-aw-compile: runs-on: ubuntu-latest timeout-minutes: 5 diff --git a/scripts/verify-persona-teams.sh b/scripts/verify-persona-teams.sh new file mode 100755 index 000000000..813a95455 --- /dev/null +++ b/scripts/verify-persona-teams.sh @@ -0,0 +1,159 @@ +#!/usr/bin/env bash +set -euo pipefail +# verify-persona-teams.sh — LIVE persona addressing-team check (#1280). +# +# The persona standard (§4.1) requires a persona's `address.handle` to name an org +# TEAM with specific LIVE properties. The hermetic personas/validate-personas.py +# checks the handle SHAPE (team slug == id) but deliberately cannot reach the +# network. This script is the separate, network-touching half: for every +# personas//persona.yml that declares an `address` block, it verifies the live +# team named by `address.handle`: +# +# 1. the team EXISTS, +# 2. privacy == "closed" (secret teams are unmentionable), +# 3. notification_setting == "notifications_disabled" (routes a webhook, not a page). +# +# The slug==id invariant is already enforced hermetically by validate-personas.py +# and is NOT re-checked here — this check is about LIVE state only. A persona with +# no `address` block passes (address is optional unless the `mention` surface is on). +# +# FAIL CLOSED: any API/parse error is a failure, never a pass. Conflating "could +# not read the team" with "the team is fine" is the exact systemic bug this check +# exists to prevent (petry-projects/.github#755, finding 3). If the team cannot be +# read, the check fails. +# +# Requires: gh (authenticated with an ORG-scoped token — github.token cannot read +# teams; GH_PAT_WORKFLOWS is available in CI), jq, and python3 + PyYAML. Diagnostics +# use the `::error::persona manifest invalid: ...` style of validate-personas.py. +# +# Usage: verify-persona-teams.sh [personas_root] (default: personas) +# VPT_ORG — org whose teams are read (default: petry-projects) + +export VPT_ORG="${VPT_ORG:-petry-projects}" + +vpt_fail() { + echo "::error::persona manifest invalid: $*" >&2 + return 1 +} + +# vpt_list_addresses — print a "\t" line for every +# personas//persona.yml under that declares an address.handle. Personas +# with no `address` block are omitted (they pass). A malformed handle (not an +# 'org/team-slug' string) is a hard error — printed as a `::error::` diagnostic and +# a non-zero exit, never a traceback. +vpt_list_addresses() { + local root="$1" + python3 - "$root" <<'PY' +import glob +import os +import sys + +try: + import yaml +except ImportError: + print("::error::persona manifest invalid: PyYAML not installed (pip install pyyaml)", + file=sys.stderr) + sys.exit(1) + +vpt_org = os.environ.get("VPT_ORG", "petry-projects") +root = sys.argv[1] +for manifest in sorted(glob.glob(os.path.join(root, "*", "persona.yml"))): + try: + with open(manifest, encoding="utf-8") as fh: + data = yaml.safe_load(fh) + except (OSError, yaml.YAMLError) as exc: + print(f"::error::persona manifest invalid: could not read/parse {manifest}: {exc}", + file=sys.stderr) + sys.exit(1) + if not isinstance(data, dict): + continue + address = data.get("address") + if address is None: + continue + if not isinstance(address, dict): + print(f"::error::persona manifest invalid: {manifest}: address must be a mapping, " + f"got {address!r}", file=sys.stderr) + sys.exit(1) + handle = address.get("handle") + if not isinstance(handle, str) or "/" not in handle: + print(f"::error::persona manifest invalid: {manifest}: address.handle must be an " + f"'org/team-slug' string (e.g. 'petry-projects/qa-lead'), got {handle!r}", + file=sys.stderr) + sys.exit(1) + org, slug = handle.split("/", 1) + if org != vpt_org: + print(f"::error::persona manifest invalid: {manifest}: address.handle org '{org}' " + f"does not match expected org '{vpt_org}' — cross-org handles cannot be verified", + file=sys.stderr) + sys.exit(1) + print(f"{slug}\t{manifest}") +PY +} + +# vpt_team_api — echo the JSON for org team , or return non-zero on +# any API error (a nonexistent team returns HTTP 404, which is such an error). +# Isolated so the hermetic test suite can stub `gh`. +vpt_team_api() { + local slug="$1" + gh api "orgs/${VPT_ORG}/teams/${slug}" +} + +# vpt_check_team — fetch and verify the live team. Fails closed: +# a read/parse error, a secret team, or an enabled notification setting all fail. +vpt_check_team() { + local manifest="$1" slug="$2" json privacy notif + if ! json="$(vpt_team_api "$slug")"; then + vpt_fail "$manifest: could not read team '${VPT_ORG}/${slug}' from the org teams API — failing closed (an API error is never treated as a valid team). The team may not exist, or the token may not be org-scoped (use GH_PAT_WORKFLOWS)." || return 1 + fi + if ! privacy="$(jq -er '.privacy?' <<< "$json")"; then + vpt_fail "$manifest: team '${VPT_ORG}/${slug}' API response has no 'privacy' field (or is not JSON) — failing closed." || return 1 + fi + if ! notif="$(jq -er '.notification_setting?' <<< "$json")"; then + vpt_fail "$manifest: team '${VPT_ORG}/${slug}' API response has no 'notification_setting' field (or is not JSON) — failing closed." || return 1 + fi + if [ "$privacy" != "closed" ]; then + vpt_fail "$manifest: team '${VPT_ORG}/${slug}' has privacy '${privacy}', expected 'closed' — a secret team is unmentionable, so every mention silently no-ops." || return 1 + fi + if [ "$notif" != "notifications_disabled" ]; then + vpt_fail "$manifest: team '${VPT_ORG}/${slug}' has notification_setting '${notif}', expected 'notifications_disabled' — the handle routes a webhook, it must not page the team's members." || return 1 + fi + echo "verify-persona-teams: OK — ${manifest} → ${VPT_ORG}/${slug} (exists, closed, notifications disabled)." +} + +vpt_run() { + local root="${1:-personas}" + command -v gh >/dev/null 2>&1 || vpt_fail "gh is required but not installed" + command -v jq >/dev/null 2>&1 || vpt_fail "jq is required but not installed" + command -v python3 >/dev/null 2>&1 || vpt_fail "python3 is required but not installed" + + if [ ! -d "$root" ]; then + echo "verify-persona-teams: no personas root at '${root}' — nothing to verify." + return 0 + fi + + local lines + if ! lines="$(vpt_list_addresses "$root")"; then + # vpt_list_addresses already printed a ::error:: diagnostic; fail closed. + return 1 + fi + + if [ -z "$lines" ]; then + echo "verify-persona-teams: no personas declare an address block — nothing to verify." + return 0 + fi + + local count=0 slug manifest + while IFS=$'\t' read -r slug manifest; do + [ -n "$slug" ] || continue + vpt_check_team "$manifest" "$slug" || return 1 + count=$((count + 1)) + done <<< "$lines" + + echo "verify-persona-teams: OK — ${count} persona team(s) verified live." +} + +# Only run main when executed directly, so the hermetic tests can source the +# functions and stub `gh`. +if [ "${BASH_SOURCE[0]}" = "${0}" ]; then + vpt_run "$@" +fi diff --git a/tests/verify_persona_teams.bats b/tests/verify_persona_teams.bats new file mode 100644 index 000000000..25508d183 --- /dev/null +++ b/tests/verify_persona_teams.bats @@ -0,0 +1,177 @@ +#!/usr/bin/env bats +# Hermetic unit + fixture tests for scripts/verify-persona-teams.sh (#1280). +# +# verify-persona-teams.sh is the LIVE, network-touching half of persona addressing +# validation: for every personas//persona.yml that declares an `address` block, +# it verifies the team named by address.handle EXISTS, is `privacy: closed` (secret +# teams are unmentionable), and is `notification_setting: notifications_disabled` +# (the handle routes a webhook, it must not page members). It FAILS CLOSED — an API +# error is never read as "the team is fine" (#755). +# +# These tests stay hermetic (the sibling of the hermetic validate-personas suite): +# the network call goes through `gh`, which is stubbed here to return canned team +# JSON or an error. NO real teams API is ever contacted. +# +# Run with: bats tests/verify_persona_teams.bats + +setup() { + REPO_ROOT="$(cd "$(dirname "$BATS_TEST_FILENAME")/.." && pwd)" + SCRIPT="$REPO_ROOT/scripts/verify-persona-teams.sh" + TMP="$BATS_TEST_TMPDIR" + # shellcheck source=/dev/null + source "$SCRIPT" + + # Stub gh so vpt_team_api never touches the network. It emulates + # gh api orgs//teams/: echo VPT_TEST_TEAM_JSON, or fail if + # VPT_TEST_GH_FAIL=1 (an API/HTTP error, e.g. 404 for a nonexistent team). + gh() { + if [ "${VPT_TEST_GH_FAIL:-0}" = "1" ]; then + return 1 + fi + printf '%s' "${VPT_TEST_TEAM_JSON:-}" + } + export -f gh +} + +# make_persona [handle] — write a minimal personas//persona.yml fixture. +# With a handle, it carries an `address` block; without, it has none. +make_persona() { + local id="$1" handle="${2:-}" + mkdir -p "$TMP/personas/$id" + { + printf 'id: %s\nname: %s\n' "$id" "$id" + if [ -n "$handle" ]; then + printf 'address:\n handle: %s\n aliases: []\n' "$handle" + fi + } > "$TMP/personas/$id/persona.yml" +} + +# --------------------------------------------------------------------------- +# vpt_check_team — verify one live team (the core assertion, fails closed) +# --------------------------------------------------------------------------- + +@test "vpt_check_team passes a closed, notifications-disabled team" { + export VPT_TEST_TEAM_JSON='{"slug":"qa-lead","privacy":"closed","notification_setting":"notifications_disabled"}' + run vpt_check_team "personas/qa-lead/persona.yml" qa-lead + [ "$status" -eq 0 ] +} + +@test "vpt_check_team fails a secret team (unmentionable)" { + export VPT_TEST_TEAM_JSON='{"slug":"qa-lead","privacy":"secret","notification_setting":"notifications_disabled"}' + run vpt_check_team "personas/qa-lead/persona.yml" qa-lead + [ "$status" -ne 0 ] + [[ "$output" == *"privacy"* ]] + [[ "$output" == *"closed"* ]] + [[ "$output" != *"Traceback"* ]] +} + +@test "vpt_check_team fails a team that pages its members" { + export VPT_TEST_TEAM_JSON='{"slug":"qa-lead","privacy":"closed","notification_setting":"notifications_enabled"}' + run vpt_check_team "personas/qa-lead/persona.yml" qa-lead + [ "$status" -ne 0 ] + [[ "$output" == *"notification_setting"* ]] + [[ "$output" != *"Traceback"* ]] +} + +@test "vpt_check_team fails CLOSED when the teams API errors" { + export VPT_TEST_GH_FAIL=1 + run vpt_check_team "personas/qa-lead/persona.yml" qa-lead + [ "$status" -ne 0 ] + [[ "$output" == *"could not read team"* ]] + [[ "$output" != *"Traceback"* ]] +} + +@test "vpt_check_team fails a team JSON missing privacy" { + export VPT_TEST_TEAM_JSON='{"slug":"qa-lead","notification_setting":"notifications_disabled"}' + run vpt_check_team "personas/qa-lead/persona.yml" qa-lead + [ "$status" -ne 0 ] + [[ "$output" != *"Traceback"* ]] +} + +@test "vpt_check_team fails when the API returns non-JSON" { + export VPT_TEST_TEAM_JSON='not json at all' + run vpt_check_team "personas/qa-lead/persona.yml" qa-lead + [ "$status" -ne 0 ] + [[ "$output" != *"Traceback"* ]] +} + +# --------------------------------------------------------------------------- +# vpt_list_addresses — extract (slug, manifest) only for address-carrying personas +# --------------------------------------------------------------------------- + +@test "vpt_list_addresses emits slug+manifest only for personas with an address" { + make_persona qa-lead petry-projects/qa-lead + make_persona no-addr + run vpt_list_addresses "$TMP/personas" + [ "$status" -eq 0 ] + [[ "$output" == *"qa-lead"* ]] + [[ "$output" != *"no-addr"* ]] +} + +@test "vpt_list_addresses reports a malformed handle without a traceback" { + make_persona bad "no-slash-handle" + run vpt_list_addresses "$TMP/personas" + [ "$status" -ne 0 ] + [[ "$output" != *"Traceback"* ]] +} + +@test "vpt_list_addresses fails a persona with an empty address block" { + mkdir -p "$TMP/personas/empty-addr" + printf 'id: empty-addr\nname: empty-addr\naddress: {}\n' > "$TMP/personas/empty-addr/persona.yml" + run vpt_list_addresses "$TMP/personas" + [ "$status" -ne 0 ] + [[ "$output" != *"Traceback"* ]] +} + +@test "vpt_list_addresses fails a handle whose org does not match VPT_ORG" { + make_persona wrong-org other-org/qa-lead + run vpt_list_addresses "$TMP/personas" + [ "$status" -ne 0 ] + [[ "$output" == *"other-org"* ]] + [[ "$output" != *"Traceback"* ]] +} + +# --------------------------------------------------------------------------- +# vpt_run — end-to-end orchestration over a fixture personas tree +# --------------------------------------------------------------------------- + +@test "vpt_run verifies a persona whose team is closed + notifications-disabled" { + make_persona qa-lead petry-projects/qa-lead + export VPT_TEST_TEAM_JSON='{"slug":"qa-lead","privacy":"closed","notification_setting":"notifications_disabled"}' + run vpt_run "$TMP/personas" + [ "$status" -eq 0 ] + [[ "$output" == *"OK"* ]] +} + +@test "vpt_run fails when a persona's team is secret" { + make_persona qa-lead petry-projects/qa-lead + export VPT_TEST_TEAM_JSON='{"slug":"qa-lead","privacy":"secret","notification_setting":"notifications_disabled"}' + run vpt_run "$TMP/personas" + [ "$status" -ne 0 ] +} + +@test "vpt_run fails CLOSED when the teams API errors for a declared address" { + make_persona qa-lead petry-projects/qa-lead + export VPT_TEST_GH_FAIL=1 + run vpt_run "$TMP/personas" + [ "$status" -ne 0 ] + [[ "$output" == *"could not read team"* ]] +} + +@test "vpt_run passes a persona with no address block (address is optional)" { + make_persona no-addr + run vpt_run "$TMP/personas" + [ "$status" -eq 0 ] +} + +@test "vpt_run is a no-op when there is no personas root" { + run vpt_run "$TMP/nonexistent" + [ "$status" -eq 0 ] +} + +@test "vpt_run reports a malformed handle without a traceback" { + make_persona bad "no-slash-handle" + run vpt_run "$TMP/personas" + [ "$status" -ne 0 ] + [[ "$output" != *"Traceback"* ]] +}