From 028d64aa581465476cda996da3354f058a89fb10 Mon Sep 17 00:00:00 2001 From: "Flemming N. Larsen" <2179727+flemming-n-larsen@users.noreply.github.com> Date: Tue, 20 Oct 2026 22:10:29 +0200 Subject: [PATCH 1/2] feat: automate safe bot moderation and merge --- .github/workflows/manual-moderation.yml | 31 ++++ .github/workflows/moderate.yml | 75 ++++++++ .github/workflows/publish-catalog.yml | 8 +- .github/workflows/validate.yml | 4 +- CODEOWNERS | 10 +- CONTRIBUTING.md | 2 +- GOVERNANCE.md | 14 +- README.md | 4 +- moderation/BattleSmoke.java | 31 ++++ moderation/assess.py | 228 ++++++++++++++++++++++++ moderation/moderate.py | 152 ++++++++++++++++ tests/test_moderation.py | 153 ++++++++++++++++ 12 files changed, 698 insertions(+), 14 deletions(-) create mode 100644 .github/workflows/manual-moderation.yml create mode 100644 .github/workflows/moderate.yml create mode 100644 moderation/BattleSmoke.java create mode 100644 moderation/assess.py create mode 100644 moderation/moderate.py create mode 100644 tests/test_moderation.py diff --git a/.github/workflows/manual-moderation.yml b/.github/workflows/manual-moderation.yml new file mode 100644 index 0000000..c7f2020 --- /dev/null +++ b/.github/workflows/manual-moderation.yml @@ -0,0 +1,31 @@ +name: Resolve escalated moderation + +on: + issue_comment: + types: [created] + +jobs: + resolve: + if: ${{ github.event.issue.pull_request && github.event.comment.user.login == 'flemming-n-larsen' && startsWith(github.event.comment.body, '/moderation approve ') }} + 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: Resolve only the current pending commit + env: + GH_TOKEN: ${{ steps.app.outputs.token }} + PR_NUMBER: ${{ github.event.issue.number }} + COMMENT_ID: ${{ github.event.comment.id }} + run: | + HEAD_SHA="$(gh api "repos/${GITHUB_REPOSITORY}/pulls/${PR_NUMBER}" --jq .head.sha)" + python moderation/moderate.py --pr "$PR_NUMBER" --sha "$HEAD_SHA" --comment-id "$COMMENT_ID" diff --git a/.github/workflows/moderate.yml b/.github/workflows/moderate.yml new file mode 100644 index 0000000..031f409 --- /dev/null +++ b/.github/workflows/moderate.yml @@ -0,0 +1,75 @@ +name: Moderate bot submissions + +on: + pull_request_target: + types: [opened, synchronize, reopened, ready_for_review] + branches: [main] + +concurrency: + group: moderate-${{ github.event.pull_request.number }} + cancel-in-progress: true + +jobs: + assess: + runs-on: ubuntu-latest + timeout-minutes: 15 + permissions: + contents: read + pull-requests: read + outputs: + kind: ${{ steps.assess.outputs.kind }} + reason_b64: ${{ steps.assess.outputs.reason_b64 }} + steps: + - uses: actions/checkout@v4 + with: + persist-credentials: false + fetch-depth: 0 + - name: Fetch and pin the untrusted PR as data + env: + PR_NUMBER: ${{ github.event.pull_request.number }} + HEAD_SHA: ${{ github.event.pull_request.head.sha }} + run: | + set -euo pipefail + [[ "$PR_NUMBER" =~ ^[0-9]+$ && "$HEAD_SHA" =~ ^[0-9a-f]{40}$ ]] + git fetch --no-tags origin "+refs/pull/${PR_NUMBER}/head:refs/remotes/origin/moderation-${PR_NUMBER}" + [[ "$(git rev-parse "refs/remotes/origin/moderation-${PR_NUMBER}")" == "$HEAD_SHA" ]] + - name: Pull pinned runtime image + run: docker pull ghcr.io/robocode-dev/rumble-client@sha256:32ab3259f0ce69f319174b54ba36f4d85949c1a30ea4e65a94010b9c931acf2d + - name: Assess source and run isolated battle + id: assess + env: + BASE_SHA: ${{ github.sha }} + HEAD_SHA: ${{ github.event.pull_request.head.sha }} + PR_OWNER: ${{ github.event.pull_request.user.login }} + run: | + python moderation/assess.py --base "$BASE_SHA" --head "$HEAD_SHA" --owner "$PR_OWNER" --output "$RUNNER_TEMP/moderation.json" + + verdict: + needs: assess + if: always() + runs-on: ubuntu-latest + timeout-minutes: 5 + 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 verdict to the exact PR head + env: + GH_TOKEN: ${{ steps.app.outputs.token }} + PR_NUMBER: ${{ github.event.pull_request.number }} + HEAD_SHA: ${{ github.event.pull_request.head.sha }} + BASE_SHA: ${{ github.event.pull_request.base.sha }} + VERDICT_KIND: ${{ needs.assess.outputs.kind }} + VERDICT_REASON_B64: ${{ needs.assess.outputs.reason_b64 }} + RUN_URL: ${{ github.server_url }}/${{ github.repository }}/actions/runs/${{ github.run_id }} + run: | + python moderation/moderate.py --pr "$PR_NUMBER" --sha "$HEAD_SHA" --base-sha "$BASE_SHA" --kind "$VERDICT_KIND" --reason-b64 "$VERDICT_REASON_B64" --run-url "$RUN_URL" diff --git a/.github/workflows/publish-catalog.yml b/.github/workflows/publish-catalog.yml index aca1fca..d510723 100644 --- a/.github/workflows/publish-catalog.yml +++ b/.github/workflows/publish-catalog.yml @@ -15,7 +15,7 @@ on: permissions: contents: write pull-requests: read - checks: read + statuses: read concurrency: group: publish-catalog @@ -56,8 +56,8 @@ jobs: head_sha="$(jq -r '.head.sha' <<< "$pr")" owner="$(jq -r '.user.login' <<< "$pr")" [[ "$head_sha" =~ ^[0-9a-f]{40}$ && "$owner" =~ ^[A-Za-z0-9-]+$ ]] || { echo "Unexpected pull request identity." >&2; exit 1; } - successful_validation="$(gh api "repos/${GITHUB_REPOSITORY}/commits/${head_sha}/check-runs" --jq '[.check_runs[] | select(.name == "validate" and .app.slug == "github-actions" and .conclusion == "success")] | length')" - [[ "$successful_validation" -gt 0 ]] || { echo "The merged pull request has no successful validate check for its head commit." >&2; exit 1; } + moderation_state="$(gh api "repos/${GITHUB_REPOSITORY}/commits/${head_sha}/status" --jq '[.statuses[] | select(.context == "bot-moderation")][0].state')" + [[ "$moderation_state" == "success" ]] || { echo "The merged pull request has no current successful bot-moderation status for its head commit." >&2; exit 1; } { echo "pull_request_number=$pr_number" echo "head_sha=$head_sha" @@ -70,7 +70,7 @@ jobs: - uses: actions/setup-python@v5 with: python-version: '3.12' - # The separate pull_request validation ran bot smoke without write access. This publisher + # The separate moderation job ran the submitted bot without write access. This publisher # runs only on trusted main and performs static catalog generation; it never launches bot code. - name: Regenerate catalog without running bots env: diff --git a/.github/workflows/validate.yml b/.github/workflows/validate.yml index ac90cc1..6e12edb 100644 --- a/.github/workflows/validate.yml +++ b/.github/workflows/validate.yml @@ -3,12 +3,14 @@ name: Validate bot submission on: pull_request: paths: - - bots/** - scripts/** - templates/** - tests/** - .github/workflows/validate.yml - .github/workflows/publish-catalog.yml + - .github/workflows/moderate.yml + - .github/workflows/manual-moderation.yml + - moderation/** permissions: contents: read diff --git a/CODEOWNERS b/CODEOWNERS index 7919e7b..cd7d5d9 100644 --- a/CODEOWNERS +++ b/CODEOWNERS @@ -1 +1,9 @@ -* @flemming-n-larsen +# Human review is required for automation, policy, templates, and publisher inputs. +/.github/ @flemming-n-larsen +/moderation/ @flemming-n-larsen +/scripts/ @flemming-n-larsen +/templates/ @flemming-n-larsen +/CODEOWNERS @flemming-n-larsen +/GOVERNANCE.md @flemming-n-larsen +/CONTRIBUTING.md @flemming-n-larsen +/README.md @flemming-n-larsen diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index ad3a886..a329197 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -6,7 +6,7 @@ Each bot directory's `license` field is its license grant for the complete direc The first merged pull request for a bot name reserves that name for its submitting forge account. Only that account, or an account later registered in `bots/owners.json` by an already registered account, may submit later versions. A source change requires a version bump; only the latest version remains active. Each owner has five active entries by default, including a TwinDuel team. -Use `python scripts/validate_bot.py --root . --owner --smoke` before opening a pull request. CI is authoritative. A green check does not replace moderator review, especially for a first submission. +Use `python scripts/validate_bot.py --root . --owner --smoke` before opening a pull request. CI is authoritative. A bot-only PR can be merged automatically after source and security checks and a completed one-round battle. Keep launcher scripts faithful to the published starter; modified launchers, team metadata, security uncertainty and unrelated file changes go to Flemming. If the bot does not run, the PR receives a changes-requested review explaining why; push a fix to rerun moderation. Start from the [runnable templates and copy-and-rename instructions](templates/README.md). They include pinned dependencies, both launchers and the required `RUMBLE_SMOKE_READY ` branch. The validator requires Python 3.11+ and `sh`; Windows users can install Git for Windows and make its shell available. It sets `RUMBLE_SMOKE=1`, invokes each individual bot's `.sh` launcher and requires a successful exit with the marker within 20 seconds. This verifies startup rather than a server handshake. Commit each `.sh` with executable mode `100755`, using `git update-index --chmod=+x` after staging it. Team entries contain JSON only and skip individual-bot launcher checks. diff --git a/GOVERNANCE.md b/GOVERNANCE.md index 5186f71..eeb00a1 100644 --- a/GOVERNANCE.md +++ b/GOVERNANCE.md @@ -1,19 +1,23 @@ # Governance -The `robocode-dev` organization owns this repository. At least three moderators should retain ownership and CODEOWNERS review rights before ranked submissions open. One moderator approval and successful validation are required for a new bot; trusted owners may receive auto-merge eligibility for version bumps after repeated clean submissions. +The `robocode-dev` organization owns this repository. Flemming is currently the human moderator; recruiting at least three people with ownership and CODEOWNERS review rights remains a future goal, not a condition for accepting ranked submissions. GitHub moderation automatically approves and merges a new bot or version update only when the pull request changes one bot directory, the source and dependency checks find no restricted behavior, and the bot completes one round against a trusted reference. A failed bot receives a changes-requested review with a diagnostic. Security uncertainty, infrastructure failure, TwinDuel team metadata, and non-bot changes stay blocked for Flemming's decision. Moderators maintain `bots/banned.json` through reviewed pull requests. A ban can cover an account or a bot and can be temporary. CI prevents a banned account from submitting and excludes disqualified bots from the generated catalog. Facts in the future `rumble-data` repository are never deleted merely because an entry is disqualified. Moderators resolve name-squatting, lost-account recovery, license complaints, and appeal requests case by case. A credible copyright complaint removes the bot from the working tree and disqualifies it pending resolution. -Once each quarter, a moderator performs a fork drill: fork the repository, enable Actions, run validation, and regenerate the catalog without credentials other than the forge-provided token. Record the outcome in a GitHub issue. +Once each quarter, a moderator performs a fork drill: fork the repository, enable Actions, run validation, and regenerate the catalog. Record the outcome in a GitHub issue. A fork can run the source validator without moderator credentials; automatic review and merge require the installed moderation App. ## GitHub Actions permissions -The pull-request validation workflow requests `contents: read`. Public fork pull requests receive a read-only `GITHUB_TOKEN` even if a workflow asks for write access, and GitHub may require a maintainer with write permission to approve a first-time contributor's workflow run. Approve the validation run only after checking the proposed source; never grant the validation workflow write access to the upstream repository. +The `pull_request_target` moderation workflow runs trusted code from `main` for every pull request. Its assessment job has a read-only token and no secrets. It fetches the exact PR commit as data, checks eligibility and source, then runs one real Battle Runner round in a pinned, non-root container with a read-only bot mount, limited resources, no capabilities and no GitHub credentials. Its separate verdict job never checks out or executes PR content. The repository's Actions event policy must explicitly allow this trusted workflow's `pull_request_target` event. Restrict this policy to this workflow; never run untrusted source in the verdict job. -After a bot pull request is merged, the publisher runs on a push to `main` that changes `bots/**`. It uses the associated merged pull request to check that the submission's `validate` check passed, then checks out trusted `main` and runs static catalog generation without `--smoke`. The publisher must never execute a submitted bot launcher with its write token. Its `GITHUB_TOKEN` needs `contents: write`, `pull-requests: read`, and `checks: read`, as declared in `.github/workflows/publish-catalog.yml`; it uses no personal access token or repository secret. +Install a dedicated GitHub App on **only** `robocode-dev/rumble-bots`, with Contents, Pull requests, Commit statuses and Issues repository permissions set to write. Store its ID and private key as `RUMBLE_MODERATOR_APP_ID` and `RUMBLE_MODERATOR_PRIVATE_KEY` repository secrets. The verdict job obtains a short-lived installation token. It approves and merges only the tested head SHA after the required `bot-moderation` status succeeds; new commits invalidate that result. A failed bot gets a failing status and `REQUEST_CHANGES`. Ambiguous and non-bot PRs receive a pending status and a comment naming Flemming and the reason. Flemming may review a pending PR and comment `/moderation approve ` to make its current head green; Flemming still merges that PR manually. Do not add the moderation App to branch-rule bypasses. + +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. The existing catalog publisher pushes generated files to `main`, so allow **only** `github-actions[bot]` to bypass that rule for its trusted publisher workflow. Keep its `contents: write` token confined to that workflow. 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. + +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. 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. -GitHub may hold a fork contributor's validation run for approval by a maintainer with write access. The publisher does not need `pull_request_target` or a privileged fork workflow. On a `main` push, it 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 the `main` branch with its merged pull-request number. The dispatch verifies that it merged into `main` and that its head commit has a successful `validate` check 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. diff --git a/README.md b/README.md index fa5c6bd..457d19b 100644 --- a/README.md +++ b/README.md @@ -11,11 +11,11 @@ See the [bot author guide](https://robocode.dev/rumble/bot-author-guide) for the 3. Declare one permitted SPDX license: `MIT`, `Apache-2.0`, `BSD-3-Clause`, or `GPL-3.0-or-later`. 4. Run `python scripts/validate_bot.py --root . --owner --smoke` before opening a pull request. -Read [CONTRIBUTING.md](CONTRIBUTING.md) for the full submission, ownership, licensing, and moderation rules. The [template guide](templates/README.md#smoke-check-and-executable-scripts) explains smoke markers, runtime prerequisites and executable bits. JSON-only TwinDuel teams use the [team template](templates/README.md#json-only-twinduel-team). The validator's smoke check starts the source entry point; it does not establish a server connection or completed battle. Connecting the smoke check to a temporary Tank Royale server is the next hardening increment before public ranked submissions open. +Read [CONTRIBUTING.md](CONTRIBUTING.md) for the full submission, ownership, licensing, and moderation rules. The [template guide](templates/README.md#smoke-check-and-executable-scripts) explains smoke markers, runtime prerequisites and executable bits. JSON-only TwinDuel teams use the [team template](templates/README.md#json-only-twinduel-team). Local smoke checks only prove source startup; GitHub moderation runs an isolated, complete one-round battle before a clean bot-only PR can be merged automatically. ## 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. -This repository is designed to be forkable: its validator is standard-library Python. Pull-request validation runs bot smoke checks with read-only permissions; after merge, a trusted `main` publisher verifies the merged pull request passed validation and regenerates catalog data without launching bot code. See [moderator governance](GOVERNANCE.md#github-actions-permissions) for required Actions permissions and recovery. +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 governance](GOVERNANCE.md#github-actions-permissions) for required Actions permissions and recovery. Source-only catalog for ranked Tank Royale Rumble bots diff --git a/moderation/BattleSmoke.java b/moderation/BattleSmoke.java new file mode 100644 index 0000000..a16cde5 --- /dev/null +++ b/moderation/BattleSmoke.java @@ -0,0 +1,31 @@ +import dev.robocode.tankroyale.runner.BattleResults; +import dev.robocode.tankroyale.runner.BattleRunner; +import dev.robocode.tankroyale.runner.BattleSetup; +import dev.robocode.tankroyale.runner.BotEntry; + +import java.nio.file.Path; +import java.util.List; + +/** Runs submitted source against a trusted reference through the real embedded server. */ +public final class BattleSmoke { + public static void main(String[] args) { + if (args.length != 4) { + throw new IllegalArgumentException("Expected submitted path, reference path, bot name and version"); + } + var setup = BattleSetup.oneVsOne(builder -> { + builder.setNumberOfRounds(1); + builder.setReadyTimeoutMicros(10_000_000); + }); + BattleResults results; + try (BattleRunner runner = BattleRunner.create(builder -> builder.embeddedServer())) { + results = runner.runBattle(setup, List.of( + BotEntry.of(Path.of(args[0])), BotEntry.of(Path.of(args[1])))); + } + if (results.getNumberOfRounds() != 1 || results.getResults().size() != 2 || + results.getResults().stream().noneMatch(result -> + result.getName().equals(args[2]) && result.getVersion().equals(args[3]))) { + throw new IllegalStateException("The submitted bot did not complete one full round"); + } + System.out.println("RUMBLE_BATTLE_READY " + args[2] + " " + args[3]); + } +} diff --git a/moderation/assess.py b/moderation/assess.py new file mode 100644 index 0000000..19c64e5 --- /dev/null +++ b/moderation/assess.py @@ -0,0 +1,228 @@ +#!/usr/bin/env python3 +"""Assess an exact PR commit using trusted code and a credential-free battle container.""" + +from __future__ import annotations + +import argparse +import base64 +import json +import os +import re +import subprocess +import sys +import tempfile +from dataclasses import dataclass +from pathlib import Path + +ROOT = Path(__file__).resolve().parents[1] +sys.path.insert(0, str(ROOT)) +from scripts.validate_bot import ( # noqa: E402 + PLATFORMS, + ValidationError, + bot_directories, + check_governance, + resolve_teams, + validate_bot, +) + +IMAGE = "ghcr.io/robocode-dev/rumble-client@sha256:32ab3259f0ce69f319174b54ba36f4d85949c1a30ea4e65a94010b9c931acf2d" +STATUS = "bot-moderation" +RISK_PATTERNS = ( + r"\b(?:subprocess|ctypes|socket|importlib|__import__|eval|exec|compile)\b", + r"\b(?:ProcessBuilder|Runtime\.getRuntime|System\.exit|java\.net|java\.nio\.file|Class\.forName|Unsafe)\b", + r"\b(?:System\.Diagnostics\.Process|DllImport|NativeLibrary|System\.Net|System\.IO\.File|Assembly\.Load)\b", + r"\b(?:child_process|node:net|node:http|node:https|require\s*\(|fetch\s*\(|Function\s*\()", + r"\b(?:open\s*\(|os\.popen|os\.system|pathlib\.Path\s*\()", +) +SOURCE_EXTENSIONS = {".py", ".java", ".cs", ".ts", ".js", ".mjs", ".cjs"} +AUTO_MERGE_SUFFIXES = SOURCE_EXTENSIONS | {".json", ".sh", ".cmd", ".csproj", ".md", ".txt"} + + +@dataclass(frozen=True) +class Verdict: + kind: str # clean, reject, escalate + reason: str + + +def git(*args: str) -> str: + return subprocess.check_output(["git", *args], cwd=ROOT, text=True).strip() + + +def changed_files(base: str, head: str) -> list[str]: + data = subprocess.check_output( + ["git", "diff", "--name-only", "--no-renames", "-z", f"{base}...{head}"], cwd=ROOT + ) + return [name.decode("utf-8") for name in data.split(b"\0") if name] + + +def classify_paths(paths: list[str]) -> tuple[str | None, str]: + if not paths: + return None, "PR contains no changed files" + directories: set[str] = set() + for path in paths: + parts = path.split("/") + if len(parts) < 4 or parts[0] != "bots" or parts[1] not in PLATFORMS: + return None, f"Non-bot or protected file changed: {path}" + directories.add("/".join(parts[:3])) + if len(directories) != 1: + return None, "A PR must change exactly one bot directory for automatic merge" + return directories.pop(), "" + + +def safe_tree(head: str) -> None: + tree = subprocess.check_output(["git", "ls-tree", "-r", "-z", head], cwd=ROOT) + for entry in tree.split(b"\0"): + if not entry: + continue + metadata, name = entry.split(b"\t", 1) + mode = metadata.split(b" ", 1)[0] + if mode not in (b"100644", b"100755"): + raise ValidationError(f"Unsafe repository object ({mode.decode()}): {name.decode('utf-8', 'replace')}") + + +def scan_source(directory: Path) -> list[str]: + issues: list[str] = [] + for source in directory.rglob("*"): + if source.suffix.lower() not in SOURCE_EXTENSIONS or not source.is_file(): + continue + for pattern in RISK_PATTERNS: + if re.search(pattern, source.read_text(encoding="utf-8"), flags=re.IGNORECASE): + issues.append(f"{source.name}: restricted API pattern {pattern}") + break + return issues + + +def scan_files(directory: Path) -> list[str]: + issues: list[str] = [] + total = 0 + for path in directory.rglob("*"): + if not path.is_file(): + continue + size = path.stat().st_size + total += size + if size > 512_000 or total > 2_000_000: + issues.append(f"{path.name}: source size exceeds automatic review limit") + if path.suffix.lower() not in AUTO_MERGE_SUFFIXES and path.name != "LICENSE": + issues.append(f"{path.name}: file type needs human review") + if path.suffix.lower() in {".sh", ".cmd"} and path.name != directory.name + path.suffix: + issues.append(f"{path.name}: extra launcher needs human review") + return issues + + +def check_launchers(platform: str, name: str, directory: Path) -> list[str]: + template = ROOT / "templates" / platform / "StarterBot" + issues: list[str] = [] + for suffix in (".sh", ".cmd"): + expected = (template / f"StarterBot{suffix}").read_text(encoding="utf-8").replace("StarterBot", name) + actual = (directory / f"{name}{suffix}").read_text(encoding="utf-8") + if actual.replace("\r\n", "\n") != expected.replace("\r\n", "\n"): + issues.append(f"{name}{suffix} differs from the trusted source-run launcher") + return issues + + +def run_battle(directory: Path, name: str, version: str) -> Verdict: + reference = "TrackFire" if name == "Fire" else "Fire" + submitted = f"/submitted/{name}" + reference_path = f"/reference/{reference}" + mounts = [ + (directory, submitted), + (ROOT / "bots" / "java" / reference, reference_path), + (ROOT / "moderation" / "BattleSmoke.java", "/harness/BattleSmoke.java"), + ] + command = [ + "docker", "run", "--rm", "--pull=never", "--read-only", "--user", "10001:10001", + "--cpus=2", "--memory=2g", "--pids-limit=512", "--cap-drop=ALL", + "--security-opt=no-new-privileges", "--tmpfs", "/tmp:rw,exec,nosuid,size=1024m", + "--env", "HOME=/tmp", "--env", "RUMBLE_PYTHON=/opt/rumble-python/bin/python", + "--env", "DOTNET_CLI_HOME=/tmp", "--env", "DOTNET_NOLOGO=1", + "--env", "JAVA_TOOL_OPTIONS=-XX:ActiveProcessorCount=2", + "--env", "npm_config_ignore_scripts=true", "--entrypoint", "java", + ] + for source, target in mounts: + command.extend(["--mount", f"type=bind,src={source.resolve()},dst={target},readonly"]) + command.extend([IMAGE, "--class-path", "/opt/rumble-client/lib/*", "/harness/BattleSmoke.java", + submitted, reference_path, name, version]) + try: + result = subprocess.run(command, text=True, capture_output=True, timeout=240, check=False) + except (OSError, subprocess.TimeoutExpired) as error: + return Verdict("escalate", f"Battle infrastructure did not finish: {error}") + if result.returncode != 0: + output = (result.stderr or result.stdout).strip()[-1200:] + if result.returncode in (125, 126, 127): + return Verdict("escalate", f"Battle container could not start: {output}") + if "Failed to start the native thread" in result.stderr or "pthread_create failed" in result.stderr: + return Verdict("escalate", f"Battle runner exhausted its process limit: {output}") + return Verdict("reject", f"One-round battle failed: {output}") + if f"RUMBLE_BATTLE_READY {name} {version}" not in result.stdout: + return Verdict("escalate", "Battle finished without the expected completion proof") + return Verdict("clean", f"{name} {version} completed one round against {reference}; source and security checks passed") + + +def assess(base: str, head: str, owner: str) -> Verdict: + if not re.fullmatch(r"[0-9a-f]{40}", base) or not re.fullmatch(r"[0-9a-f]{40}", head): + return Verdict("escalate", "Invalid Git revision in PR event") + if not re.fullmatch(r"[A-Za-z0-9-]{1,39}", owner): + return Verdict("escalate", "Invalid PR author login") + paths = changed_files(base, head) + bot_path, reason = classify_paths(paths) + if not bot_path: + return Verdict("escalate", reason) + try: + safe_tree(head) + except ValidationError as error: + return Verdict("escalate", str(error)) + with tempfile.TemporaryDirectory(prefix="rumble-pr-") as checkout: + subprocess.run(["git", "worktree", "add", "--detach", checkout, head], cwd=ROOT, + check=True, stdout=subprocess.DEVNULL) + try: + pr_root = Path(checkout) + directory = pr_root / bot_path + if not directory.is_dir(): + return Verdict("reject", f"Submitted bot directory is missing: {bot_path}") + platform = bot_path.split("/")[1] + name = directory.name + try: + bots = resolve_teams([validate_bot(key, path, smoke=False) + for key, path in bot_directories(pr_root)]) + check_governance(bots, pr_root, owner) + except (ValidationError, OSError, UnicodeError) as error: + if "source contains restricted construct" in str(error): + return Verdict("escalate", str(error)) + return Verdict("reject", str(error)) + target = next((bot for bot in bots if bot.directory == directory), None) + if target is None: + return Verdict("reject", f"No bot found at {bot_path}") + if target.team_member_identities: + return Verdict("escalate", "TwinDuel team metadata needs human review") + issues = scan_files(directory) + scan_source(directory) + check_launchers(platform, name, directory) + if issues: + return Verdict("escalate", "; ".join(issues)[:1200]) + return run_battle(directory, name, str(target.config["version"])) + finally: + subprocess.run(["git", "worktree", "remove", "--force", checkout], cwd=ROOT, + check=True, stdout=subprocess.DEVNULL) + + +def main() -> int: + parser = argparse.ArgumentParser() + parser.add_argument("--base", required=True) + parser.add_argument("--head", required=True) + parser.add_argument("--owner", required=True) + parser.add_argument("--output", type=Path, required=True) + args = parser.parse_args() + try: + verdict = assess(args.base, args.head, args.owner) + except Exception as error: # infrastructure failures must never become an approval + verdict = Verdict("escalate", f"Moderation infrastructure failed: {error}") + args.output.write_text(json.dumps(verdict.__dict__) + "\n", encoding="utf-8") + output = os.environ.get("GITHUB_OUTPUT") + if output: + with Path(output).open("a", encoding="utf-8") as handle: + handle.write(f"kind={verdict.kind}\n") + handle.write("reason_b64=" + base64.b64encode(verdict.reason.encode()).decode() + "\n") + print(f"{verdict.kind}: {verdict.reason}") + return 0 + + +if __name__ == "__main__": + raise SystemExit(main()) diff --git a/moderation/moderate.py b/moderation/moderate.py new file mode 100644 index 0000000..55d5e33 --- /dev/null +++ b/moderation/moderate.py @@ -0,0 +1,152 @@ +#!/usr/bin/env python3 +"""Apply a moderation verdict without ever checking out or executing PR content.""" + +from __future__ import annotations + +import argparse +import base64 +import json +import os +import re +import sys +from urllib.error import HTTPError +from urllib.request import Request, urlopen + +CONTEXT = "bot-moderation" +MODERATOR = "flemming-n-larsen" + + +class GitHubError(Exception): + pass + + +def api(method: str, path: str, payload: dict | None = None): + token = os.environ["GH_TOKEN"] + data = json.dumps(payload).encode() if payload is not None else None + request = Request( + f"https://api.github.com/repos/{os.environ['GITHUB_REPOSITORY']}/{path}", + data=data, method=method, + headers={"Authorization": f"Bearer {token}", "Accept": "application/vnd.github+json", + "X-GitHub-Api-Version": "2026-03-10", "Content-Type": "application/json"}, + ) + try: + with urlopen(request, timeout=20) as response: + body = response.read() + return json.loads(body) if body else None + except HTTPError as error: + raise GitHubError(f"GitHub {method} {path}: HTTP {error.code}: {error.read()[:400].decode(errors='replace')}") from error + + +def status(sha: str, state: str, description: str, run_url: str = "") -> None: + api("POST", f"statuses/{sha}", { + "state": state, "context": CONTEXT, "description": description[:140], + "target_url": run_url or None, + }) + + +def comment(number: int, body: str) -> None: + api("POST", f"issues/{number}/comments", {"body": body}) + + +def current_pr(number: int, sha: str, base_sha: str = "") -> dict | None: + pr = api("GET", f"pulls/{number}") + if (pr["state"] != "open" or pr["base"]["ref"] != "main" + or pr["head"]["sha"] != sha + or (base_sha and pr["base"]["sha"] != base_sha)): + return None + return pr + + +def automated(number: int, sha: str, kind: str, reason: str, run_url: str, base_sha: str = "") -> None: + pr = current_pr(number, sha) + if pr is None: + print("PR closed, retargeted, or updated; stale verdict ignored") + return + if base_sha and pr["base"]["sha"] != base_sha: + status(sha, "pending", "Base changed during moderation", run_url) + 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"}: + kind = "escalate" + reason = "Draft PR or missing verdict: " + reason + if kind == "escalate": + status(sha, "pending", "Flemming must review this PR", run_url) + comment(number, f"@{MODERATOR} Moderation needs your decision for `{sha[:12]}`: {reason}\n\nThe required check stays pending. After reviewing, use `/moderation approve ` or request changes.") + return + if kind == "reject": + status(sha, "failure", "Bot validation or battle failed", run_url) + api("POST", f"pulls/{number}/reviews", { + "event": "REQUEST_CHANGES", "commit_id": sha, "body": f"The bot cannot be accepted: {reason}", + }) + return + # 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}", + }) + 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) + if current_pr(number, sha, base_sha) is None: + print("PR changed after status; merge skipped") + return + try: + result = api("PUT", f"pulls/{number}/merge", {"sha": sha, "merge_method": "merge"}) + if not result or not result.get("merged"): + raise GitHubError("GitHub did not confirm the merge") + except GitHubError as error: + status(sha, "error", "Automatic merge needs maintainer attention", run_url) + comment(number, f"@{MODERATOR} The bot passed moderation, but automatic merge failed for `{sha[:12]}`: {error}") + raise + + +def human_override(number: int, sha: str, comment_id: int) -> None: + pr = current_pr(number, sha) + if pr is None: + raise GitHubError("PR is closed, retargeted, or has a newer commit") + entry = api("GET", f"issues/comments/{comment_id}") + body = entry["body"].strip() + match = re.fullmatch(r"/moderation approve\s+(.{10,500})", body, re.DOTALL) + if entry["user"]["login"].casefold() != MODERATOR or not match: + raise GitHubError("Only Flemming may approve an escalated PR with a stated reason") + combined = api("GET", f"commits/{sha}/status") + moderation = next((item for item in combined["statuses"] if item["context"] == CONTEXT), None) + if moderation is None or moderation["state"] != "pending": + raise GitHubError("Only a pending human escalation may be approved manually") + status(sha, "success", "Human moderation by Flemming") + comment(number, f"Flemming accepted the escalated moderation for `{sha[:12]}`: {match.group(1).strip()}\n\nMerge remains a human action.") + + +def main() -> int: + parser = argparse.ArgumentParser() + parser.add_argument("--pr", required=True, type=int) + parser.add_argument("--sha", required=True) + parser.add_argument("--base-sha", default="") + parser.add_argument("--kind", default="escalate") + parser.add_argument("--reason-b64", default="") + parser.add_argument("--run-url", default="") + parser.add_argument("--comment-id", type=int) + args = parser.parse_args() + if (args.pr < 1 or not re.fullmatch(r"[0-9a-f]{40}", args.sha) + or (args.base_sha and not re.fullmatch(r"[0-9a-f]{40}", args.base_sha))): + parser.error("expected a positive PR number and full head SHA") + if args.comment_id is not None: + human_override(args.pr, args.sha, args.comment_id) + else: + try: + reason = base64.b64decode(args.reason_b64, validate=True).decode("utf-8") + except (ValueError, UnicodeError) as error: + reason = f"Malformed validator output: {error}" + args.kind = "escalate" + automated(args.pr, args.sha, args.kind, reason, args.run_url, args.base_sha) + return 0 + + +if __name__ == "__main__": + try: + raise SystemExit(main()) + except GitHubError as error: + print(error, file=sys.stderr) + raise SystemExit(1) from error diff --git a/tests/test_moderation.py b/tests/test_moderation.py new file mode 100644 index 0000000..77ae96d --- /dev/null +++ b/tests/test_moderation.py @@ -0,0 +1,153 @@ +"""Focused checks for bot-only eligibility and exact-commit moderation decisions.""" + +import tempfile +import unittest +from pathlib import Path +from subprocess import CompletedProcess +from unittest.mock import patch + +from moderation import assess, moderate + + +SHA = "a" * 40 + + +class SubmissionEligibilityTest(unittest.TestCase): + def test_one_bot_directory_is_eligible(self): + self.assertEqual( + assess.classify_paths(["bots/python/NewBot/NewBot.json", "bots/python/NewBot/src/NewBot.py"]), + ("bots/python/NewBot", ""), + ) + + def test_mixed_or_protected_paths_go_to_human(self): + for paths in ( + ["bots/python/NewBot/src/NewBot.py", ".github/workflows/moderate.yml"], + ["bots/index.json"], + ["bots/banned.json"], + ["bots/python/One/One.py", "bots/python/Two/Two.py"], + ): + with self.subTest(paths=paths): + self.assertIsNone(assess.classify_paths(paths)[0]) + + def test_restricted_source_and_extra_launcher_need_human(self): + with tempfile.TemporaryDirectory() as temp: + bot = Path(temp) / "Example" + bot.mkdir() + (bot / "Example.py").write_text("import subprocess\n", encoding="utf-8") + (bot / "extra.sh").write_text("#!/bin/sh\n", encoding="utf-8") + self.assertTrue(assess.scan_source(bot)) + self.assertTrue(assess.scan_files(bot)) + + def test_official_starters_have_reviewable_launchers(self): + for platform in assess.PLATFORMS: + with self.subTest(platform=platform): + bot = assess.ROOT / "templates" / platform / "StarterBot" + self.assertEqual(assess.check_launchers(platform, "StarterBot", bot), []) + self.assertEqual(assess.scan_source(bot), []) + self.assertEqual(assess.scan_files(bot), []) + + def test_container_failure_escalates_but_bot_failure_requests_changes(self): + bot = assess.ROOT / "templates" / "java" / "StarterBot" + for exit_code, expected in ((125, "escalate"), (1, "reject")): + with self.subTest(exit_code=exit_code), patch.object( + assess.subprocess, "run", return_value=CompletedProcess([], exit_code, "", "failure") + ): + self.assertEqual(assess.run_battle(bot, "StarterBot", "1.0.0").kind, expected) + + +class ExactCommitDecisionTest(unittest.TestCase): + def setUp(self): + self.pr = {"state": "open", "base": {"ref": "main", "sha": "c" * 40}, "head": {"sha": SHA}, "draft": False} + + def test_clean_verdict_approves_and_merges_only_the_tested_sha(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, "clean", "battle passed", "https://example.test/run") + review = next(payload for method, path, payload in calls if path == "pulls/12/reviews") + merge = next(payload for method, path, payload in calls if path == "pulls/12/merge") + self.assertEqual(review["event"], "APPROVE") + self.assertEqual(review["commit_id"], SHA) + self.assertEqual(merge["sha"], SHA) + + 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: + moderate.automated(12, SHA, "clean", "battle passed", "") + self.assertEqual(api.call_count, 1) + + def test_changed_base_cannot_receive_a_stale_verdict(self): + with patch.object(moderate, "api", return_value=self.pr) as api: + moderate.automated(12, SHA, "clean", "battle passed", "", "d" * 40) + self.assertFalse(any(call.args[1] == "pulls/12/merge" for call in api.call_args_list)) + self.assertTrue(any(call.args[1] == f"statuses/{SHA}" and call.args[2]["state"] == "pending" + for call in api.call_args_list)) + + def test_runtime_failure_requests_changes_and_blocks(self): + calls = [] + + def fake_api(method, path, payload=None): + calls.append((method, path, payload)) + return self.pr if path == "pulls/12" else {} + + with patch.object(moderate, "api", side_effect=fake_api): + moderate.automated(12, SHA, "reject", "bot timed out", "") + self.assertTrue(any(path == "pulls/12/reviews" and payload["event"] == "REQUEST_CHANGES" + for _, path, payload in calls)) + self.assertFalse(any(path == "pulls/12/merge" for _, path, _ in calls)) + + def test_uncertain_result_requires_flemming(self): + calls = [] + + def fake_api(method, path, payload=None): + calls.append((method, path, payload)) + return self.pr if path == "pulls/12" else {} + + with patch.object(moderate, "api", side_effect=fake_api): + moderate.automated(12, SHA, "escalate", "suspicious network code", "") + self.assertTrue(any(path == f"statuses/{SHA}" and payload["state"] == "pending" + for _, path, payload in calls)) + self.assertTrue(any(path == "issues/12/comments" and "@flemming-n-larsen" in payload["body"] + for _, path, payload in calls)) + self.assertFalse(any(path == "pulls/12/merge" for _, path, _ in calls)) + + def test_only_flemming_can_clear_a_pending_escalation(self): + calls = [] + + def fake_api(method, path, payload=None): + calls.append((method, path, payload)) + if path == "pulls/12": + return self.pr + if path == "issues/comments/42": + return {"body": "/moderation approve Reviewed the extra source", "user": {"login": "flemming-n-larsen"}} + if path == f"commits/{SHA}/status": + return {"statuses": [{"context": moderate.CONTEXT, "state": "pending"}]} + return {} + + with patch.object(moderate, "api", side_effect=fake_api): + moderate.human_override(12, SHA, 42) + self.assertTrue(any(path == f"statuses/{SHA}" and payload["state"] == "success" + for _, path, payload in calls)) + self.assertFalse(any(path == "pulls/12/merge" for _, path, _ in calls)) + + def test_other_commenter_cannot_clear_an_escalation(self): + def fake_api(method, path, payload=None): + if path == "pulls/12": + return self.pr + if path == "issues/comments/42": + return {"body": "/moderation approve Looks good to me", "user": {"login": "someone-else"}} + raise AssertionError("No status or merge may be written") + + with patch.object(moderate, "api", side_effect=fake_api): + with self.assertRaises(moderate.GitHubError): + moderate.human_override(12, SHA, 42) + + +if __name__ == "__main__": + unittest.main() From 005f557f8061bacc39d8556b1414f6a0918ce3ae Mon Sep 17 00:00:00 2001 From: "Flemming N. Larsen" <2179727+flemming-n-larsen@users.noreply.github.com> Date: Tue, 20 Oct 2026 22:12:26 +0200 Subject: [PATCH 2/2] docs: protect catalog inputs and preserve maintainer integration --- CODEOWNERS | 3 +++ GOVERNANCE.md | 2 +- 2 files changed, 4 insertions(+), 1 deletion(-) diff --git a/CODEOWNERS b/CODEOWNERS index cd7d5d9..716b2e7 100644 --- a/CODEOWNERS +++ b/CODEOWNERS @@ -7,3 +7,6 @@ /GOVERNANCE.md @flemming-n-larsen /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 eeb00a1..77f206b 100644 --- a/GOVERNANCE.md +++ b/GOVERNANCE.md @@ -14,7 +14,7 @@ The `pull_request_target` moderation workflow runs trusted code from `main` for Install a dedicated GitHub App on **only** `robocode-dev/rumble-bots`, with Contents, Pull requests, Commit statuses and Issues repository permissions set to write. Store its ID and private key as `RUMBLE_MODERATOR_APP_ID` and `RUMBLE_MODERATOR_PRIVATE_KEY` repository secrets. The verdict job obtains a short-lived installation token. It approves and merges only the tested head SHA after the required `bot-moderation` status succeeds; new commits invalidate that result. A failed bot gets a failing status and `REQUEST_CHANGES`. Ambiguous and non-bot PRs receive a pending status and a comment naming Flemming and the reason. Flemming may review a pending PR and comment `/moderation approve ` to make its current head green; Flemming still merges that PR manually. Do not add the moderation App to branch-rule bypasses. -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. The existing catalog publisher pushes generated files to `main`, so allow **only** `github-actions[bot]` to bypass that rule for its trusted publisher workflow. Keep its `contents: write` token confined to that workflow. 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. +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. The existing catalog publisher pushes generated files to `main`, so allow `github-actions[bot]` to bypass that rule for its trusted publisher workflow. 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. Keep the publisher's `contents: write` token confined to that workflow. 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. 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.