From b6f3ffa304c5a143f05ce9beef5ab2a60829be6b Mon Sep 17 00:00:00 2001 From: "Flemming N. Larsen" <2179727+flemming-n-larsen@users.noreply.github.com> Date: Wed, 21 Oct 2026 00:03:39 +0200 Subject: [PATCH 1/3] Publish generated bot catalogs through verified PRs --- .github/workflows/publish-catalog.yml | 25 +++++- .github/workflows/validate.yml | 1 + .github/workflows/verify-catalog.yml | 66 +++++++++++++++ CODEOWNERS | 2 - GOVERNANCE.md | 8 +- README.md | 2 +- moderation/moderate.py | 6 +- moderation/verify_catalog.py | 114 ++++++++++++++++++++++++++ scripts/validate_bot.py | 22 +++-- tests/test_moderation.py | 88 +++++++++++++++++++- tests/test_validate_bot.py | 13 +++ 11 files changed, 327 insertions(+), 20 deletions(-) create mode 100644 .github/workflows/verify-catalog.yml create mode 100644 moderation/verify_catalog.py diff --git a/.github/workflows/publish-catalog.yml b/.github/workflows/publish-catalog.yml index d510723..e726477 100644 --- a/.github/workflows/publish-catalog.yml +++ b/.github/workflows/publish-catalog.yml @@ -5,6 +5,9 @@ on: branches: [main] paths: - 'bots/**' + - '!bots/index.json' + - '!bots/owners.json' + - '!bots/banned.json' workflow_dispatch: inputs: pull_request_number: @@ -14,7 +17,7 @@ on: permissions: contents: write - pull-requests: read + pull-requests: write statuses: read concurrency: @@ -75,13 +78,27 @@ jobs: - name: Regenerate catalog without running bots env: BOT_OWNER: ${{ steps.submission.outputs.owner }} - run: python scripts/validate_bot.py --root . --owner "$BOT_OWNER" --generate - - name: Commit and publish generated catalog + run: GITHUB_SHA="$(git rev-parse HEAD)" python scripts/validate_bot.py --root . --owner "$BOT_OWNER" --generate + - name: Open generated catalog pull request + env: + GH_TOKEN: ${{ github.token }} + SOURCE_PR: ${{ steps.submission.outputs.pull_request_number }} run: | + set -euo pipefail git config user.name 'github-actions[bot]' git config user.email '41898282+github-actions[bot]@users.noreply.github.com' git add bots/index.json bots/owners.json if ! git diff --cached --quiet; then git commit -m 'chore: regenerate bot catalog' - git push origin HEAD:main + branch="catalog/pr-${SOURCE_PR}-${GITHUB_RUN_ID}-${GITHUB_RUN_ATTEMPT}" + git push origin "HEAD:refs/heads/${branch}" + pr_number="$(gh api "repos/${GITHUB_REPOSITORY}/pulls" \ + -f title="chore: publish bot catalog after #${SOURCE_PR}" \ + -f head="$branch" -f base=main \ + -f body="Generated catalog for merged bot submission #${SOURCE_PR}. The moderator App verifies the exact generated files before merging." \ + --jq .number)" + gh api "repos/${GITHUB_REPOSITORY}/dispatches" \ + -f event_type=catalog_publication \ + -F "client_payload[pull_request_number]=${pr_number}" \ + -F "client_payload[source_pull_request_number]=${SOURCE_PR}" fi diff --git a/.github/workflows/validate.yml b/.github/workflows/validate.yml index 6e12edb..e358ba7 100644 --- a/.github/workflows/validate.yml +++ b/.github/workflows/validate.yml @@ -8,6 +8,7 @@ on: - tests/** - .github/workflows/validate.yml - .github/workflows/publish-catalog.yml + - .github/workflows/verify-catalog.yml - .github/workflows/moderate.yml - .github/workflows/manual-moderation.yml - moderation/** diff --git a/.github/workflows/verify-catalog.yml b/.github/workflows/verify-catalog.yml new file mode 100644 index 0000000..8d1d5d6 --- /dev/null +++ b/.github/workflows/verify-catalog.yml @@ -0,0 +1,66 @@ +name: Verify generated bot catalog + +on: + repository_dispatch: + types: [catalog_publication] + +jobs: + verify: + runs-on: ubuntu-latest + permissions: + contents: read + pull-requests: read + statuses: read + outputs: + kind: ${{ steps.verify.outputs.kind }} + reason_b64: ${{ steps.verify.outputs.reason_b64 }} + sha: ${{ steps.verify.outputs.sha }} + base_sha: ${{ steps.verify.outputs.base_sha }} + steps: + - uses: actions/checkout@v4 + with: + fetch-depth: 0 + persist-credentials: false + - uses: actions/setup-python@v5 + with: + python-version: '3.12' + - name: Regenerate and compare catalog on trusted main + id: verify + env: + GH_TOKEN: ${{ github.token }} + CATALOG_PR: ${{ github.event.client_payload.pull_request_number }} + SOURCE_PR: ${{ github.event.client_payload.source_pull_request_number }} + run: | + set -euo pipefail + [[ "$CATALOG_PR" =~ ^[0-9]+$ && "$SOURCE_PR" =~ ^[0-9]+$ ]] + python -m moderation.verify_catalog --pr "$CATALOG_PR" --source-pr "$SOURCE_PR" --output "$RUNNER_TEMP/catalog-verdict.json" + jq -r 'to_entries[] | "\(.key)=\(.value)"' "$RUNNER_TEMP/catalog-verdict.json" >> "$GITHUB_OUTPUT" + + verdict: + needs: verify + if: ${{ always() && needs.verify.outputs.sha != '' }} + runs-on: ubuntu-latest + permissions: + contents: read + steps: + - uses: actions/checkout@v4 + with: + persist-credentials: false + - uses: actions/create-github-app-token@fee1f7d63c2ff003460e3d139729b119787bc349 + id: app + with: + app-id: ${{ secrets.RUMBLE_MODERATOR_APP_ID }} + private-key: ${{ secrets.RUMBLE_MODERATOR_PRIVATE_KEY }} + owner: robocode-dev + repositories: rumble-bots + - name: Apply exact-commit catalog verdict + env: + GH_TOKEN: ${{ steps.app.outputs.token }} + CATALOG_PR: ${{ github.event.client_payload.pull_request_number }} + HEAD_SHA: ${{ needs.verify.outputs.sha }} + BASE_SHA: ${{ needs.verify.outputs.base_sha }} + VERDICT_KIND: ${{ needs.verify.outputs.kind }} + VERDICT_REASON_B64: ${{ needs.verify.outputs.reason_b64 }} + RUN_URL: ${{ github.server_url }}/${{ github.repository }}/actions/runs/${{ github.run_id }} + run: | + python moderation/moderate.py --pr "$CATALOG_PR" --sha "$HEAD_SHA" --base-sha "$BASE_SHA" --kind "$VERDICT_KIND" --reason-b64 "$VERDICT_REASON_B64" --run-url "$RUN_URL" diff --git a/CODEOWNERS b/CODEOWNERS index 716b2e7..fde0542 100644 --- a/CODEOWNERS +++ b/CODEOWNERS @@ -8,5 +8,3 @@ /CONTRIBUTING.md @flemming-n-larsen /README.md @flemming-n-larsen /bots/banned.json @flemming-n-larsen -/bots/index.json @flemming-n-larsen -/bots/owners.json @flemming-n-larsen diff --git a/GOVERNANCE.md b/GOVERNANCE.md index b220364..16bb6a3 100644 --- a/GOVERNANCE.md +++ b/GOVERNANCE.md @@ -42,10 +42,10 @@ Install a dedicated GitHub App on **only** `robocode-dev/rumble-bots`, with Cont Protect `main` with a required `bot-moderation` status from the moderation App, a pull-request requirement, and code-owner review for the protected paths listed in `CODEOWNERS`. Bot directories have no human code owner, so a clean bot can merge after the App's approval. Flemming needs a pull-request-only bypass for policy changes he authors, because GitHub will not count an author's approval of their own pull request; use this exception only after human review of the change. Do not grant any bypass to the moderation App. Verify the App, status source, branch rule and publisher on a test PR before enabling auto-merge. The automatic security screen is a conservative filter, not proof that arbitrary bot source is harmless; clients still execute bots in their own sandbox. -**Deployment hold:** The current catalog publisher pushes generated files directly to `main` using `GITHUB_TOKEN`. GitHub rejected `github-actions[bot]` as a ruleset bypass actor with HTTP 422 (`Actor GitHub Actions integration must be part of the ruleset source or owner organization`). No required-status branch ruleset is active yet. The publisher must move to a protected integration path before this rule can be enabled; until then, do not claim that a failing bot is blocked from manual merge by GitHub. +**Deployment hold:** GitHub rejected `github-actions[bot]` as a ruleset bypass actor with HTTP 422 (`Actor GitHub Actions integration must be part of the ruleset source or owner organization`). No required-status branch ruleset is active yet. The generated catalog PR path must pass a live test before the rule is enabled; until then, do not claim that a failing bot is blocked from manual merge by GitHub. -After a bot pull request is merged by the App, the publisher runs on a push to `main` that changes `bots/**`. It checks that the merged PR's exact head had a successful `bot-moderation` status, then checks out trusted `main` and runs static catalog generation without executing a submitted launcher. Its `GITHUB_TOKEN` needs `contents: write`, `pull-requests: read`, and `statuses: read`, as declared in `.github/workflows/publish-catalog.yml`; it uses no personal access token or repository secret. +After a bot pull request is merged by the App, the publisher runs on a push to `main` that changes `bots/**`. It checks that the merged PR's exact head had a successful `bot-moderation` status, then checks out trusted `main` and runs static catalog generation without executing a submitted launcher. It pushes only a `catalog/pr-*` branch and opens a pull request as `github-actions[bot]`. Because a pull request created with `GITHUB_TOKEN` does not start the moderator workflow, it dispatches the separate catalog verification workflow. That workflow reads the pull request as data without executing its contents, checks its source submission and exact generated files against trusted `main`, then uses the moderator App to approve and merge only the verified head. The publisher's `GITHUB_TOKEN` needs `contents: write`, `pull-requests: write`, and `statuses: read`; it uses no personal access token or repository secret. Generated catalog files have no CODEOWNERS entry because the App's verified approval satisfies the review requirement; policy and workflow files remain human-owned. -For publication to work, GitHub Actions must be enabled and the repository or organization policy must permit these token scopes. The repository's **Settings → Actions → General → Workflow permissions** should allow read and write permissions for `GITHUB_TOKEN` (currently enabled for this repository); the workflow itself still grants only the scopes listed above. If `main` is protected by branch rules, allow `github-actions[bot]` to push the generated catalog files, or publishing will fail. Recheck these settings if organization policy or branch rules change. +For publication to work, GitHub Actions must be enabled and the repository or organization policy must permit these token scopes and the `repository_dispatch` verification event. The repository's **Settings → Actions → General → Workflow permissions** must allow read and write permissions for `GITHUB_TOKEN` (currently enabled) and **Allow GitHub Actions to create and approve pull requests** (currently disabled; enable before live publication testing). The latter repository-wide setting is required for `GITHUB_TOKEN` to create the generated PR even though the publisher never approves one; only the moderator App approves it. No bot account needs a direct-push bypass on `main`. Recheck these settings if organization policy or branch rules change. -On a `main` push, the publisher retries the commit-to-PR lookup for up to 55 seconds to allow GitHub to publish the association. To recover a merged submission whose publication still failed, a maintainer can run **Publish bot catalog** from `main` with its merged PR number. The dispatch verifies that it merged into `main` and that its head commit passed moderation before it regenerates the catalog using the PR author's account as owner. +On a `main` push, the publisher retries the commit-to-PR lookup for up to 55 seconds to allow GitHub to publish the association. To recover a merged submission whose publication still failed, a maintainer can run **Publish bot catalog** from `main` with its merged PR number. The dispatch verifies that it merged into `main` and that its head commit passed moderation before it regenerates the catalog using the PR author's account as owner. If the target branch advances during verification, the catalog PR remains blocked and the publisher must rerun against current `main`. diff --git a/README.md b/README.md index 5e5c2f6..88fa405 100644 --- a/README.md +++ b/README.md @@ -15,7 +15,7 @@ Read [CONTRIBUTING.md](CONTRIBUTING.md) for the full submission, ownership, lice ## Catalog -`bots/index.json` and `bots/owners.json` are generated by CI and must never be edited in a pull request. A catalog entry contains the bot identity, owner, source path, source-tree SHA-256, active lifecycle status, and `teamMembers`. Individual bots publish an empty member list; a TwinDuel team publishes exactly two active ` ` member identities, so a member release always forces the team entry to be resubmitted. A team directory contains nothing but its own JSON, its members may live under different platform directories, and its published platform is the members' shared platform or `Mixed`. Only `active` entries are eligible for matchmaking. +`bots/index.json` and `bots/owners.json` are generated by CI. Bot authors must not edit them; only the trusted catalog publisher opens generated-file pull requests, which the moderator App verifies before merge. A catalog entry contains the bot identity, owner, source path, source-tree SHA-256, active lifecycle status, and `teamMembers`. Individual bots publish an empty member list; a TwinDuel team publishes exactly two active ` ` member identities, so a member release always forces the team entry to be resubmitted. A team directory contains nothing but its own JSON, its members may live under different platform directories, and its published platform is the members' shared platform or `Mixed`. Only `active` entries are eligible for matchmaking. This repository is designed to be forkable: its validator is standard-library Python. Pull-request assessment runs bot source with read-only permissions inside a restricted container; a separate trusted job makes the exact-commit review and merge decision. After merge, the `main` publisher verifies moderation and regenerates catalog data without launching bot code. See [moderator App setup](GOVERNANCE.md#moderator-app-registration) and [Actions permissions](GOVERNANCE.md#github-actions-permissions) for deployment and recovery. Source-only catalog for ranked Tank Royale Rumble bots diff --git a/moderation/moderate.py b/moderation/moderate.py index 55d5e33..5eb034a 100644 --- a/moderation/moderate.py +++ b/moderation/moderate.py @@ -67,7 +67,7 @@ def automated(number: int, sha: str, kind: str, reason: str, run_url: str, base_ comment(number, f"@{MODERATOR} The target branch changed while `{sha[:12]}` was being tested. Please review the current base and retry or resolve with `/moderation approve `.") return reason = reason.strip()[:1200] or "Moderation did not produce a reason" - if pr["draft"] or kind not in {"clean", "reject", "escalate"}: + if pr["draft"] or kind not in {"clean", "catalog", "reject", "escalate"}: kind = "escalate" reason = "Draft PR or missing verdict: " + reason if kind == "escalate": @@ -83,12 +83,12 @@ def automated(number: int, sha: str, kind: str, reason: str, run_url: str, base_ # The trusted job reads the live head again immediately before every irreversible action. api("POST", f"pulls/{number}/reviews", { "event": "APPROVE", "commit_id": sha, - "body": f"Automated moderation passed for `{sha[:12]}`: {reason}", + "body": f"Automated {'catalog verification' if kind == 'catalog' else 'moderation'} passed for `{sha[:12]}`: {reason}", }) if current_pr(number, sha, base_sha) is None: print("PR changed after approval; merge skipped") return - status(sha, "success", "Source and completed battle passed", run_url) + status(sha, "success", "Generated catalog verified" if kind == "catalog" else "Source and completed battle passed", run_url) if current_pr(number, sha, base_sha) is None: print("PR changed after status; merge skipped") return diff --git a/moderation/verify_catalog.py b/moderation/verify_catalog.py new file mode 100644 index 0000000..afe1359 --- /dev/null +++ b/moderation/verify_catalog.py @@ -0,0 +1,114 @@ +#!/usr/bin/env python3 +"""Verify a generated catalog PR against trusted main without executing PR content.""" + +from __future__ import annotations + +import argparse +import base64 +import json +import os +import re +import subprocess +from datetime import UTC, datetime, timedelta +from pathlib import Path + +from moderation.assess import classify_paths +from moderation.moderate import CONTEXT, api + +CATALOG_FILES = {"bots/index.json", "bots/owners.json"} +APP_LOGIN = "tank-royale-rumble-moderator[bot]" + + +class VerificationError(Exception): + pass + + +def git(*args: str) -> str: + result = subprocess.run(["git", *args], capture_output=True, text=True, check=True) + return result.stdout.strip() + + +def source_paths(number: int) -> list[str]: + paths = [] + page = 1 + while True: + files = api("GET", f"pulls/{number}/files?per_page=100&page={page}") + paths.extend(item["filename"] for item in files) + if len(files) < 100: + return paths + page += 1 + + +def verify(number: int, source_number: int) -> tuple[str, str]: + pr = api("GET", f"pulls/{number}") + head = pr["head"]["sha"] + base = pr["base"]["sha"] + if (pr["state"] != "open" or pr["draft"] or pr["base"]["ref"] != "main" + or pr["head"]["repo"]["full_name"] != os.environ["GITHUB_REPOSITORY"] + or pr["user"]["login"] != "github-actions[bot]" + or not re.fullmatch(rf"catalog/pr-{source_number}-[0-9]+-[0-9]+", pr["head"]["ref"])): + raise VerificationError("Not a publisher-created catalog PR on main") + if git("rev-parse", "HEAD") != base: + raise VerificationError("Main moved after catalog generation; rerun the publisher on current main") + source = api("GET", f"pulls/{source_number}") + if (source["merged_at"] is None or source["base"]["ref"] != "main" + or not re.fullmatch(r"[A-Za-z0-9-]+", source["user"]["login"])): + raise VerificationError("Source submission is not a merged main PR") + if classify_paths(source_paths(source_number))[0] is None: + raise VerificationError("Source PR did not change exactly one bot directory") + status = api("GET", f"commits/{source['head']['sha']}/status") + moderation = next((item for item in status["statuses"] if item["context"] == CONTEXT), None) + if (moderation is None or moderation["state"] != "success" + or moderation["creator"]["login"] != APP_LOGIN): + raise VerificationError("Source bot has no successful moderator App status") + git("merge-base", "--is-ancestor", source["merge_commit_sha"], base) + subprocess.run(["git", "fetch", "--no-tags", "origin", f"+refs/pull/{number}/head:refs/remotes/origin/catalog-verify"], check=True) + if git("rev-parse", "refs/remotes/origin/catalog-verify") != head: + raise VerificationError("Catalog PR head changed during verification") + if git("rev-parse", f"{head}^") != base: + raise VerificationError("Catalog commit is not based on current main") + changed = set(git("diff", "--name-only", base, head).splitlines()) + if not changed or not changed <= CATALOG_FILES: + raise VerificationError("Catalog PR changes unexpected files") + proposed = {path: git("show", f"{head}:{path}") + "\n" for path in CATALOG_FILES} + try: + generated_at = json.loads(proposed["bots/index.json"])["generatedAt"] + timestamp = datetime.fromisoformat(generated_at.replace("Z", "+00:00")) + if (timestamp.tzinfo is None or timestamp.utcoffset() != timedelta(0) + or not datetime.now(UTC) - timedelta(days=2) <= timestamp <= datetime.now(UTC) + timedelta(minutes=5)): + raise ValueError("timestamp outside publication window") + except (ValueError, KeyError, TypeError) as error: + raise VerificationError(f"Invalid generatedAt timestamp: {error}") from error + env = os.environ.copy() + env["GITHUB_SHA"] = base + result = subprocess.run( + ["python", "scripts/validate_bot.py", "--root", ".", "--owner", source["user"]["login"], + "--generate", "--generated-at", generated_at], env=env, capture_output=True, text=True, + ) + if result.returncode: + raise VerificationError(f"Trusted catalog generation failed: {result.stderr[:300]}") + for path in CATALOG_FILES: + if Path(path).read_text(encoding="utf-8") != proposed[path]: + raise VerificationError(f"Generated content differs: {path}") + return head, base + + +def main() -> None: + parser = argparse.ArgumentParser() + parser.add_argument("--pr", type=int, required=True) + parser.add_argument("--source-pr", type=int, required=True) + parser.add_argument("--output", type=Path, required=True) + args = parser.parse_args() + try: + head, base = verify(args.pr, args.source_pr) + kind, reason = "catalog", f"Exact generated catalog matches trusted main and merged bot PR #{args.source_pr}" + except (VerificationError, subprocess.CalledProcessError, KeyError, TypeError, ValueError) as error: + pr = api("GET", f"pulls/{args.pr}") + head, base = pr["head"]["sha"], pr["base"]["sha"] + kind, reason = "escalate", f"Catalog verification needs maintainer attention: {error}" + args.output.write_text(json.dumps({"kind": kind, "reason_b64": base64.b64encode(reason.encode()).decode(), + "sha": head, "base_sha": base}) + "\n", encoding="utf-8") + + +if __name__ == "__main__": + main() diff --git a/scripts/validate_bot.py b/scripts/validate_bot.py index dd7304a..098db9d 100644 --- a/scripts/validate_bot.py +++ b/scripts/validate_bot.py @@ -13,7 +13,7 @@ import unicodedata from urllib.parse import unquote from dataclasses import dataclass, replace -from datetime import UTC, datetime +from datetime import UTC, datetime, timedelta from pathlib import Path from typing import Any @@ -289,7 +289,8 @@ def check_governance(bots: list[Bot], root: Path, owner: str) -> None: raise ValidationError(f"owner `{owner}` exceeds the five active bot slot limit") -def generated_catalog(bots: list[Bot], root: Path, owner: str) -> tuple[dict[str, Any], dict[str, Any]]: +def generated_catalog(bots: list[Bot], root: Path, owner: str, generated_at: datetime | None = None) -> tuple[dict[str, Any], dict[str, Any]]: + generated_at = generated_at or datetime.now(UTC) existing_owners = read_json(root / "bots" / "owners.json") if (root / "bots" / "owners.json").exists() else {"schemaVersion": 1, "owners": []} existing_catalog = read_json(root / "bots" / "index.json") if (root / "bots" / "index.json").exists() else {"bots": []} owner_by_bot = {bot_name: record["ownerId"] for record in existing_owners.get("owners", []) for bot_name in record.get("bots", [])} @@ -319,7 +320,7 @@ def generated_catalog(bots: list[Bot], root: Path, owner: str) -> tuple[dict[str for owner_id, owner_record in sorted(owners.items()) ], } - today = datetime.now(UTC).date().isoformat() + today = generated_at.date().isoformat() current_by_name = {bot.name: bot for bot in bots} history = [] for entry in existing_catalog.get("bots", []): @@ -333,7 +334,7 @@ def generated_catalog(bots: list[Bot], root: Path, owner: str) -> tuple[dict[str previous = next((entry for entry in existing_catalog.get("bots", []) if entry.get("name") == bot.name and entry.get("version") == bot.config["version"]), None) team_members = list(bot.team_member_identities) active.append({"name": bot.name, "version": bot.config["version"], "platform": bot.platform, "path": bot.directory.relative_to(root).as_posix(), "sourceHash": bot.source_hash, "owner": owner_by_bot.get(bot.name, owner), "authors": bot.config["authors"], "addedAt": previous.get("addedAt", today) if previous else today, "status": "active", "teamMembers": team_members}) - catalog = {"schemaVersion": 1, "generatedAt": datetime.now(UTC).replace(microsecond=0).isoformat().replace("+00:00", "Z"), "commit": os.environ.get("GITHUB_SHA", "local"), "bots": history + active} + catalog = {"schemaVersion": 1, "generatedAt": generated_at.replace(microsecond=0).isoformat().replace("+00:00", "Z"), "commit": os.environ.get("GITHUB_SHA", "local"), "bots": history + active} return catalog, owner_data @@ -343,13 +344,24 @@ def main() -> int: parser.add_argument("--owner", required=True) parser.add_argument("--smoke", action="store_true") parser.add_argument("--generate", action="store_true") + parser.add_argument("--generated-at", help="UTC timestamp for reproducible catalog verification") arguments = parser.parse_args() + generated_at = None + if arguments.generated_at: + if not arguments.generate: + parser.error("--generated-at requires --generate") + try: + generated_at = datetime.fromisoformat(arguments.generated_at.replace("Z", "+00:00")) + except ValueError: + parser.error("--generated-at must be an ISO 8601 UTC timestamp") + if generated_at.tzinfo is None or generated_at.utcoffset() != timedelta(0): + parser.error("--generated-at must use UTC") root = arguments.root.resolve() try: bots = resolve_teams([validate_bot(platform, directory, smoke=arguments.smoke) for platform, directory in bot_directories(root)]) check_governance(bots, root, arguments.owner) if arguments.generate: - catalog, owners = generated_catalog(bots, root, arguments.owner) + catalog, owners = generated_catalog(bots, root, arguments.owner, generated_at) (root / "bots" / "index.json").write_text(json.dumps(catalog, indent=2) + "\n", encoding="utf-8") (root / "bots" / "owners.json").write_text(json.dumps(owners, indent=2) + "\n", encoding="utf-8") except ValidationError as error: diff --git a/tests/test_moderation.py b/tests/test_moderation.py index 77ae96d..ef218b2 100644 --- a/tests/test_moderation.py +++ b/tests/test_moderation.py @@ -2,11 +2,13 @@ import tempfile import unittest +import json +from datetime import UTC, datetime from pathlib import Path from subprocess import CompletedProcess from unittest.mock import patch -from moderation import assess, moderate +from moderation import assess, moderate, verify_catalog SHA = "a" * 40 @@ -76,6 +78,24 @@ def fake_api(method, path, payload=None): self.assertEqual(review["commit_id"], SHA) self.assertEqual(merge["sha"], SHA) + def test_verified_catalog_uses_catalog_status_and_exact_commit_review(self): + calls = [] + + def fake_api(method, path, payload=None): + calls.append((method, path, payload)) + if path == "pulls/12/merge": + return {"merged": True} + return self.pr if path == "pulls/12" else {} + + with patch.object(moderate, "api", side_effect=fake_api): + moderate.automated(12, SHA, "catalog", "generated files match", "") + self.assertTrue(any(path == f"statuses/{SHA}" and payload["description"] == "Generated catalog verified" + for _, path, payload in calls)) + self.assertTrue(any(path == "pulls/12/reviews" and payload["commit_id"] == SHA + for _, path, payload in calls)) + self.assertTrue(any(path == "pulls/12/merge" and payload["sha"] == SHA + for _, path, payload in calls)) + def test_new_commit_cannot_receive_a_stale_verdict(self): self.pr["head"]["sha"] = "b" * 40 with patch.object(moderate, "api", return_value=self.pr) as api: @@ -149,5 +169,71 @@ def fake_api(method, path, payload=None): moderate.human_override(12, SHA, 42) +class CatalogVerificationTest(unittest.TestCase): + def setUp(self): + self.pr = {"state": "open", "draft": False, "base": {"ref": "main", "sha": "c" * 40}, + "head": {"sha": SHA, "ref": "catalog/pr-7-123-1", "repo": {"full_name": "robocode-dev/rumble-bots"}}, + "user": {"login": "github-actions[bot]"}} + + def test_other_author_cannot_claim_generated_catalog(self): + self.pr["user"]["login"] = "untrusted" + with patch.dict("os.environ", {"GITHUB_REPOSITORY": "robocode-dev/rumble-bots"}), patch.object( + verify_catalog, "api", return_value=self.pr + ) as api: + with self.assertRaisesRegex(verify_catalog.VerificationError, "publisher-created"): + verify_catalog.verify(12, 7) + self.assertEqual(api.call_count, 1) + + def test_moved_main_cannot_validate_stale_catalog(self): + with patch.dict("os.environ", {"GITHUB_REPOSITORY": "robocode-dev/rumble-bots"}), patch.object( + verify_catalog, "api", return_value=self.pr + ), patch.object(verify_catalog, "git", return_value="d" * 40): + with self.assertRaisesRegex(verify_catalog.VerificationError, "Main moved"): + verify_catalog.verify(12, 7) + + def test_catalog_must_match_trusted_regeneration(self): + base = "c" * 40 + generated_at = datetime.now(UTC).replace(microsecond=0).isoformat().replace("+00:00", "Z") + proposed = {"bots/index.json": json.dumps({"generatedAt": generated_at}) + "\n", + "bots/owners.json": "{}\n"} + source = {"merged_at": generated_at, "base": {"ref": "main"}, + "head": {"sha": "d" * 40}, "merge_commit_sha": "e" * 40, + "user": {"login": "contributor"}} + + def fake_api(method, path, payload=None): + if path == "pulls/12": + return self.pr + if path == "pulls/7": + return source + if path.startswith("pulls/7/files"): + return [{"filename": "bots/python/Nova/Nova.json"}] + if path == f"commits/{'d' * 40}/status": + return {"statuses": [{"context": moderate.CONTEXT, "state": "success", + "creator": {"login": verify_catalog.APP_LOGIN}}]} + raise AssertionError(path) + + def fake_git(*args): + if args == ("rev-parse", "HEAD"): + return base + if args == ("rev-parse", "refs/remotes/origin/catalog-verify"): + return SHA + if args == ("rev-parse", f"{SHA}^"): + return base + if args[:2] == ("diff", "--name-only"): + return "bots/index.json" + if args[0] == "show": + return proposed[args[1].split(":", 1)[1]].strip() + if args[:2] == ("merge-base", "--is-ancestor"): + return "" + raise AssertionError(args) + + with patch.dict("os.environ", {"GITHUB_REPOSITORY": "robocode-dev/rumble-bots"}), patch.object( + verify_catalog, "api", side_effect=fake_api + ), patch.object(verify_catalog, "git", side_effect=fake_git), patch.object( + verify_catalog.subprocess, "run", return_value=CompletedProcess([], 0, "", "") + ), patch.object(verify_catalog.Path, "read_text", autospec=True, side_effect=lambda path, encoding=None: proposed[path.as_posix()]): + self.assertEqual((SHA, base), verify_catalog.verify(12, 7)) + + if __name__ == "__main__": unittest.main() diff --git a/tests/test_validate_bot.py b/tests/test_validate_bot.py index 413fa20..34e727e 100644 --- a/tests/test_validate_bot.py +++ b/tests/test_validate_bot.py @@ -108,6 +108,19 @@ def test_valid_submission_generates_an_active_catalog_entry(self) -> None: orbit = next(entry for entry in catalog["bots"] if entry["name"] == "Orbit" and entry["status"] == "active") self.assertEqual("active", orbit["status"]) + def test_catalog_can_be_reproduced_at_its_recorded_utc_timestamp(self) -> None: + timestamp = "2026-10-21T00:00:00Z" + self.assertEqual(0, self.run_validator("--generate", "--generated-at", timestamp).returncode) + first = (self.root / "bots" / "index.json").read_bytes() + self.assertEqual(0, self.run_validator("--generate", "--generated-at", timestamp).returncode) + self.assertEqual(first, (self.root / "bots" / "index.json").read_bytes()) + self.assertEqual(timestamp, json.loads(first)["generatedAt"]) + + def test_catalog_timestamp_must_be_utc(self) -> None: + result = self.run_validator("--generate", "--generated-at", "2026-10-21T02:00:00+02:00") + self.assertNotEqual(0, result.returncode) + self.assertIn("must use UTC", result.stderr) + def test_invalid_license_is_rejected(self) -> None: config_path = self.root / "bots" / "python" / "Orbit" / "Orbit.json" config = json.loads(config_path.read_text(encoding="utf-8")) From 5007d8cdfde58071c33a74498902fed6f8c8c3c8 Mon Sep 17 00:00:00 2001 From: "Flemming N. Larsen" <2179727+flemming-n-larsen@users.noreply.github.com> Date: Wed, 21 Oct 2026 20:08:55 +0200 Subject: [PATCH 2/3] Hold overlapping unpublished bot changes for ownership review --- GOVERNANCE.md | 2 +- moderation/verify_catalog.py | 25 ++++++++++++++++++++++--- tests/test_moderation.py | 24 +++++++++++++++++++++++- 3 files changed, 46 insertions(+), 5 deletions(-) diff --git a/GOVERNANCE.md b/GOVERNANCE.md index 16bb6a3..6998391 100644 --- a/GOVERNANCE.md +++ b/GOVERNANCE.md @@ -48,4 +48,4 @@ After a bot pull request is merged by the App, the publisher runs on a push to ` For publication to work, GitHub Actions must be enabled and the repository or organization policy must permit these token scopes and the `repository_dispatch` verification event. The repository's **Settings → Actions → General → Workflow permissions** must allow read and write permissions for `GITHUB_TOKEN` (currently enabled) and **Allow GitHub Actions to create and approve pull requests** (currently disabled; enable before live publication testing). The latter repository-wide setting is required for `GITHUB_TOKEN` to create the generated PR even though the publisher never approves one; only the moderator App approves it. No bot account needs a direct-push bypass on `main`. Recheck these settings if organization policy or branch rules change. -On a `main` push, the publisher retries the commit-to-PR lookup for up to 55 seconds to allow GitHub to publish the association. To recover a merged submission whose publication still failed, a maintainer can run **Publish bot catalog** from `main` with its merged PR number. The dispatch verifies that it merged into `main` and that its head commit passed moderation before it regenerates the catalog using the PR author's account as owner. If the target branch advances during verification, the catalog PR remains blocked and the publisher must rerun against current `main`. +On a `main` push, the publisher retries the commit-to-PR lookup for up to 55 seconds to allow GitHub to publish the association. To recover a merged submission whose publication still failed, a maintainer can run **Publish bot catalog** from `main` with its merged PR number. The dispatch verifies that it merged into `main` and that its head commit passed moderation before it regenerates the catalog using the PR author's account as owner. If the target branch advances during verification, the catalog PR remains blocked and the publisher must rerun against current `main`. If several bot submissions merge before publication finishes, verification holds the generated PR for Flemming rather than guessing which submitter owns each unpublished bot; reconcile ownership before publishing. diff --git a/moderation/verify_catalog.py b/moderation/verify_catalog.py index afe1359..e8f5cb6 100644 --- a/moderation/verify_catalog.py +++ b/moderation/verify_catalog.py @@ -13,7 +13,8 @@ from pathlib import Path from moderation.assess import classify_paths -from moderation.moderate import CONTEXT, api +from moderation.moderate import CONTEXT, GitHubError, api +from scripts.validate_bot import ValidationError, bot_directories, read_json, resolve_teams, validate_bot CATALOG_FILES = {"bots/index.json", "bots/owners.json"} APP_LOGIN = "tank-royale-rumble-moderator[bot]" @@ -39,6 +40,20 @@ def source_paths(number: int) -> list[str]: page += 1 +def unpublished_bot_paths(root: Path) -> set[str]: + catalog = read_json(root / "bots" / "index.json") + active = {entry["name"]: entry for entry in catalog["bots"] if entry["status"] == "active"} + bots = resolve_teams([validate_bot(platform, directory, smoke=False) + for platform, directory in bot_directories(root)]) + return { + bot.directory.relative_to(root).as_posix() + for bot in bots + if (entry := active.get(bot.name)) is None + or entry["version"] != bot.config["version"] + or entry["sourceHash"] != bot.source_hash + } + + def verify(number: int, source_number: int) -> tuple[str, str]: pr = api("GET", f"pulls/{number}") head = pr["head"]["sha"] @@ -54,8 +69,12 @@ def verify(number: int, source_number: int) -> tuple[str, str]: if (source["merged_at"] is None or source["base"]["ref"] != "main" or not re.fullmatch(r"[A-Za-z0-9-]+", source["user"]["login"])): raise VerificationError("Source submission is not a merged main PR") - if classify_paths(source_paths(source_number))[0] is None: + source_directory = classify_paths(source_paths(source_number))[0] + if source_directory is None: raise VerificationError("Source PR did not change exactly one bot directory") + pending_paths = unpublished_bot_paths(Path.cwd()) + if pending_paths != {source_directory}: + raise VerificationError(f"Other unpublished bot changes need ownership review: {sorted(pending_paths)}") status = api("GET", f"commits/{source['head']['sha']}/status") moderation = next((item for item in status["statuses"] if item["context"] == CONTEXT), None) if (moderation is None or moderation["state"] != "success" @@ -102,7 +121,7 @@ def main() -> None: try: head, base = verify(args.pr, args.source_pr) kind, reason = "catalog", f"Exact generated catalog matches trusted main and merged bot PR #{args.source_pr}" - except (VerificationError, subprocess.CalledProcessError, KeyError, TypeError, ValueError) as error: + except (VerificationError, ValidationError, GitHubError, subprocess.CalledProcessError, KeyError, TypeError, ValueError) as error: pr = api("GET", f"pulls/{args.pr}") head, base = pr["head"]["sha"], pr["base"]["sha"] kind, reason = "escalate", f"Catalog verification needs maintainer attention: {error}" diff --git a/tests/test_moderation.py b/tests/test_moderation.py index ef218b2..00c065e 100644 --- a/tests/test_moderation.py +++ b/tests/test_moderation.py @@ -191,6 +191,27 @@ def test_moved_main_cannot_validate_stale_catalog(self): with self.assertRaisesRegex(verify_catalog.VerificationError, "Main moved"): verify_catalog.verify(12, 7) + def test_multiple_unpublished_bots_need_ownership_review(self): + source = {"merged_at": "2026-10-21T00:00:00Z", "base": {"ref": "main"}, + "user": {"login": "contributor"}} + + def fake_api(method, path, payload=None): + if path == "pulls/12": + return self.pr + if path == "pulls/7": + return source + if path.startswith("pulls/7/files"): + return [{"filename": "bots/python/Nova/Nova.json"}] + raise AssertionError(path) + + with patch.dict("os.environ", {"GITHUB_REPOSITORY": "robocode-dev/rumble-bots"}), patch.object( + verify_catalog, "api", side_effect=fake_api + ), patch.object(verify_catalog, "git", return_value="c" * 40), patch.object( + verify_catalog, "unpublished_bot_paths", return_value={"bots/python/Nova", "bots/java/Other"} + ): + with self.assertRaisesRegex(verify_catalog.VerificationError, "Other unpublished bot changes"): + verify_catalog.verify(12, 7) + def test_catalog_must_match_trusted_regeneration(self): base = "c" * 40 generated_at = datetime.now(UTC).replace(microsecond=0).isoformat().replace("+00:00", "Z") @@ -229,7 +250,8 @@ def fake_git(*args): with patch.dict("os.environ", {"GITHUB_REPOSITORY": "robocode-dev/rumble-bots"}), patch.object( verify_catalog, "api", side_effect=fake_api - ), patch.object(verify_catalog, "git", side_effect=fake_git), patch.object( + ), patch.object(verify_catalog, "unpublished_bot_paths", return_value={"bots/python/Nova"}), patch.object( + verify_catalog, "git", side_effect=fake_git), patch.object( verify_catalog.subprocess, "run", return_value=CompletedProcess([], 0, "", "") ), patch.object(verify_catalog.Path, "read_text", autospec=True, side_effect=lambda path, encoding=None: proposed[path.as_posix()]): self.assertEqual((SHA, base), verify_catalog.verify(12, 7)) From 2470bcb8c84e5a1d8b272c9addd2e85e2799f9a4 Mon Sep 17 00:00:00 2001 From: "Flemming N. Larsen" <2179727+flemming-n-larsen@users.noreply.github.com> Date: Wed, 21 Oct 2026 20:11:08 +0200 Subject: [PATCH 3/3] Test forged catalog content is rejected --- tests/test_moderation.py | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/tests/test_moderation.py b/tests/test_moderation.py index 00c065e..569d510 100644 --- a/tests/test_moderation.py +++ b/tests/test_moderation.py @@ -217,6 +217,7 @@ def test_catalog_must_match_trusted_regeneration(self): generated_at = datetime.now(UTC).replace(microsecond=0).isoformat().replace("+00:00", "Z") proposed = {"bots/index.json": json.dumps({"generatedAt": generated_at}) + "\n", "bots/owners.json": "{}\n"} + regenerated = proposed.copy() source = {"merged_at": generated_at, "base": {"ref": "main"}, "head": {"sha": "d" * 40}, "merge_commit_sha": "e" * 40, "user": {"login": "contributor"}} @@ -253,8 +254,11 @@ def fake_git(*args): ), patch.object(verify_catalog, "unpublished_bot_paths", return_value={"bots/python/Nova"}), patch.object( verify_catalog, "git", side_effect=fake_git), patch.object( verify_catalog.subprocess, "run", return_value=CompletedProcess([], 0, "", "") - ), patch.object(verify_catalog.Path, "read_text", autospec=True, side_effect=lambda path, encoding=None: proposed[path.as_posix()]): + ), patch.object(verify_catalog.Path, "read_text", autospec=True, side_effect=lambda path, encoding=None: regenerated[path.as_posix()]): self.assertEqual((SHA, base), verify_catalog.verify(12, 7)) + regenerated["bots/owners.json"] = '{"owner": "wrong"}\n' + with self.assertRaisesRegex(verify_catalog.VerificationError, "Generated content differs"): + verify_catalog.verify(12, 7) if __name__ == "__main__":