diff --git a/.github/workflows/validate.yml b/.github/workflows/validate.yml index e358ba7..5571cfa 100644 --- a/.github/workflows/validate.yml +++ b/.github/workflows/validate.yml @@ -53,4 +53,6 @@ jobs: echo 'bots/owners.json is generated by CI and cannot be edited in a pull request.' exit 1 fi - - run: python scripts/validate_bot.py --root . --owner '${{ github.event.pull_request.user.login }}' --smoke + # This workflow covers implementation changes; bot submissions receive ownership + # checks from the moderation workflow. A merged bot may await catalog publication. + - run: python scripts/validate_bot.py --root . --owner '${{ github.event.pull_request.user.login }}' --smoke --skip-governance diff --git a/GOVERNANCE.md b/GOVERNANCE.md index ce67b63..b587d62 100644 --- a/GOVERNANCE.md +++ b/GOVERNANCE.md @@ -38,6 +38,8 @@ Create a second organization-owned GitHub App named **Tank Royale Rumble Publish Generate a private key and save the Publisher App ID as `RUMBLE_PUBLISHER_APP_ID` and its full PEM contents as `RUMBLE_PUBLISHER_PRIVATE_KEY` in this repository's Actions secrets. Never commit the PEM file. Delete the downloaded local copy after securely storing the key, and rotate it in the App settings when needed. +The organization-owned Publisher App created on 2026-10-21 has App ID `5415028` and slug `tank-royale-rumble-publisher`. GitHub reports **Contents: Read** and **Pull requests: Read & write** permissions. Both repository secrets were set on 2026-10-21; their values are not recorded here. The [first live publisher run](https://github.com/robocode-dev/rumble-bots/actions/runs/39653182830) obtained its installation token and opened [catalog PR #23](https://github.com/robocode-dev/rumble-bots/pull/23) after [test bot PR #22](https://github.com/robocode-dev/rumble-bots/pull/22) was approved and merged automatically. Catalog verification exposed a status-response issue, so completion of this live test remains pending until the corrected verifier runs. + ## GitHub Actions permissions 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. diff --git a/moderation/verify_catalog.py b/moderation/verify_catalog.py index fa9cc4f..3108d0a 100644 --- a/moderation/verify_catalog.py +++ b/moderation/verify_catalog.py @@ -76,10 +76,10 @@ def verify(number: int, source_number: int) -> tuple[str, str]: 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) + statuses = api("GET", f"commits/{source['head']['sha']}/statuses?per_page=100") + moderation = next((item for item in statuses if item["context"] == CONTEXT), None) if (moderation is None or moderation["state"] != "success" - or moderation["creator"]["login"] != APP_LOGIN): + or (moderation.get("creator") or {}).get("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) diff --git a/scripts/validate_bot.py b/scripts/validate_bot.py index 098db9d..e84d9c4 100644 --- a/scripts/validate_bot.py +++ b/scripts/validate_bot.py @@ -344,8 +344,11 @@ 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("--skip-governance", action="store_true", help="Validate all bots without attributing unpublished changes to the supplied owner") parser.add_argument("--generated-at", help="UTC timestamp for reproducible catalog verification") arguments = parser.parse_args() + if arguments.skip_governance and arguments.generate: + parser.error("--skip-governance cannot be used with --generate") generated_at = None if arguments.generated_at: if not arguments.generate: @@ -359,7 +362,8 @@ def main() -> int: 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 not arguments.skip_governance: + check_governance(bots, root, arguments.owner) if arguments.generate: 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") diff --git a/tests/test_moderation.py b/tests/test_moderation.py index 77c3d14..4cb6111 100644 --- a/tests/test_moderation.py +++ b/tests/test_moderation.py @@ -221,6 +221,8 @@ def test_catalog_must_match_trusted_regeneration(self): source = {"merged_at": generated_at, "base": {"ref": "main"}, "head": {"sha": "d" * 40}, "merge_commit_sha": "e" * 40, "user": {"login": "contributor"}} + statuses = [{"context": moderate.CONTEXT, "state": "success", + "creator": {"login": verify_catalog.APP_LOGIN}}] def fake_api(method, path, payload=None): if path == "pulls/12": @@ -229,9 +231,8 @@ def fake_api(method, path, payload=None): 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}}]} + if path == f"commits/{'d' * 40}/statuses?per_page=100": + return statuses raise AssertionError(path) def fake_git(*args): @@ -259,6 +260,13 @@ def fake_git(*args): regenerated["bots/owners.json"] = '{"owner": "wrong"}\n' with self.assertRaisesRegex(verify_catalog.VerificationError, "Generated content differs"): verify_catalog.verify(12, 7) + regenerated["bots/owners.json"] = proposed["bots/owners.json"] + statuses[0]["creator"] = {"login": "untrusted"} + with self.assertRaisesRegex(verify_catalog.VerificationError, "no successful moderator App status"): + verify_catalog.verify(12, 7) + statuses[0].pop("creator") + with self.assertRaisesRegex(verify_catalog.VerificationError, "no successful moderator App status"): + verify_catalog.verify(12, 7) if __name__ == "__main__": diff --git a/tests/test_validate_bot.py b/tests/test_validate_bot.py index 34e727e..b186f8f 100644 --- a/tests/test_validate_bot.py +++ b/tests/test_validate_bot.py @@ -160,6 +160,16 @@ def test_new_bot_from_another_owner_ignores_unchanged_catalog_entries(self) -> N nova = next(entry for entry in catalog["bots"] if entry["name"] == "Nova") self.assertEqual("alice", nova["owner"]) + def test_general_validation_does_not_claim_another_owners_unpublished_bot(self) -> None: + config_path = self.root / "bots" / "java" / "SpinBot" / "SpinBot.json" + config = json.loads(config_path.read_text(encoding="utf-8")) + config["version"] = "1.0.4" + config_path.write_text(json.dumps(config), encoding="utf-8") + self.assertIn("belongs to owner", self.run_validator(owner="alice").stderr) + result = self.run_validator("--skip-governance", owner="alice") + self.assertEqual(0, result.returncode, result.stderr) + self.assertIn("cannot be used with --generate", self.run_validator("--skip-governance", "--generate").stderr) + def test_registered_secondary_account_can_update_and_is_preserved(self) -> None: owners_path = self.root / "bots" / "owners.json" owners = json.loads(owners_path.read_text(encoding="utf-8"))