From 0747e9f700b0e26ebc3d06df85511d5c5d70379e Mon Sep 17 00:00:00 2001 From: "Takuto NAKAMURA (Kyome)" Date: Wed, 15 Jul 2026 23:12:08 +0900 Subject: [PATCH 1/4] Add CI to validate contributor runner archives Runner zips are committed by contributors and handed to RunCat Neo users as-is from the gallery download link, with nothing between the two but a maintainer reading a checklist. This checks every pull request instead. The core property is that a frame archive may contain nothing but PNG files, and that every PNG consists entirely of CRC-valid chunks ending at IEND with no trailing bytes. Together those leave no room in the archive for a payload to hide, which is a stronger guarantee than scanning for known signatures would give. On top of that the validator rejects symlinks, executables, encrypted entries, zip-slip paths, zip bombs, and data appended to the archive, then fully decodes every frame and checks it against the sizes CONTRIBUTING.md documents. Interlaced PNGs are rejected: Adam7 is pointless at 36px tall, and Pillow cannot decode the frames of an interlaced APNG at all, so accepting one would mean giving up the full-decode check for exactly the files that are unusual. Runs on pull_request rather than pull_request_target. The job feeds untrusted binaries to zlib, libpng, and Pillow's C decoders, so it is the last thing here that should hold a writable token or secrets; the read-only sandbox is the actual control. That costs the ability to post a comment, so results land as inline annotations on the offending file plus a job summary. Co-Authored-By: Claude Opus 4.8 (1M context) --- .github/scripts/requirements.txt | 1 + .github/scripts/validate_runners.py | 1002 ++++++++++++++++++++++++ .github/workflows/validate-runners.yml | 48 ++ 3 files changed, 1051 insertions(+) create mode 100644 .github/scripts/requirements.txt create mode 100644 .github/scripts/validate_runners.py create mode 100644 .github/workflows/validate-runners.yml diff --git a/.github/scripts/requirements.txt b/.github/scripts/requirements.txt new file mode 100644 index 0000000..c519fe2 --- /dev/null +++ b/.github/scripts/requirements.txt @@ -0,0 +1 @@ +pillow==11.3.0 diff --git a/.github/scripts/validate_runners.py b/.github/scripts/validate_runners.py new file mode 100644 index 0000000..20c52d9 --- /dev/null +++ b/.github/scripts/validate_runners.py @@ -0,0 +1,1002 @@ +#!/usr/bin/env python3 +"""Validate contributor-submitted runner assets. + +Runs read-only against the git index. See CONTRIBUTING.md for the rules this +enforces. The security-relevant part is that every byte of every distributed +zip is proven to belong to a well-formed PNG: the archive may contain nothing +but PNG files, and each PNG must consist entirely of CRC-valid chunks ending at +IEND with no trailing bytes. That leaves nowhere to hide a payload. +""" + +from __future__ import annotations + +import argparse +import binascii +import io +import json +import os +import re +import struct +import subprocess +import sys +import warnings +import zipfile +from dataclasses import dataclass, field +from pathlib import Path + +from PIL import Image + +Image.MAX_IMAGE_PIXELS = 1_000_000 +warnings.simplefilter("error", Image.DecompressionBombWarning) + +MAX_ZIP_BYTES = 2 * 1024 * 1024 +MAX_TOTAL_UNCOMPRESSED = 8 * 1024 * 1024 +MAX_ENTRY_UNCOMPRESSED = 1 * 1024 * 1024 +MAX_ENTRIES = 64 +MAX_RATIO = 200 +MAX_PNG_BYTES = 512 * 1024 +MAX_TEXT_CHUNK_BYTES = 4 * 1024 +MAX_JSON_BYTES = 4 * 1024 +MAX_ENTRY_NAME_LENGTH = 200 +MAX_METADATA_VALUE_LENGTH = 100 +MAX_REPORTED_UNEXPECTED = 3 + +FRAME_HEIGHT = 36 +MIN_WIDTH = 10 +MAX_WIDTH = 100 +MIN_FRAMES = 2 + +RUNNER_NAME_RE = re.compile(r"^[a-z0-9]+(?:-[a-z0-9]+)*$") +PNG_SIGNATURE = b"\x89PNG\r\n\x1a\n" +EOCD_SIGNATURE = b"PK\x05\x06" +EOCD_SIZE = 22 + +ZIP_REGULAR_FILE = 0o100000 +ZIP_DIRECTORY = 0o040000 +ZIP_SYMLINK = 0o120000 +ZIP_FORMAT_MASK = 0o170000 + +DOS_HIDDEN = 0x02 +DOS_SYSTEM = 0x04 + +REZIP_COMMAND = '`zip -r {name}-frames.zip {name}-frames -x "*.DS_Store" "__MACOSX/*"`' +REZIP_HINT = "Re-create the zip from the terminal: " + REZIP_COMMAND + +# Phrased so that one reason reads correctly for either a single entry or many +# ("`x` is macOS Finder metadata" / "`x`, `y` are macOS Finder metadata"). +JUNK_REASONS = { + "__MACOSX": "macOS Finder metadata. " + REZIP_HINT, + ".DS_Store": "macOS Finder metadata. " + REZIP_HINT, + "Thumbs.db": "Windows Explorer thumbnail cache data. Delete it and re-create the zip.", + "desktop.ini": "Windows folder-settings data. Delete it and re-create the zip.", +} +APPLEDOUBLE_REASON = "macOS AppleDouble metadata. " + REZIP_HINT +HIDDEN_REASON = "hidden. The archive must contain only the frame PNGs." + +EXPECTED_FILES = ("{name}-frames.zip", "metadata.json", "preview.png") +METADATA_KEYS = {"author", "displayName"} + + +class PngError(Exception): + pass + + +@dataclass +class Problem: + runner: str | None + file: str + message: str + + +@dataclass +class Report: + problems: list[Problem] = field(default_factory=list) + checked: list[str] = field(default_factory=list) + internal_error: bool = False + + def error(self, runner: str | None, file: str, message: str) -> None: + self.problems.append(Problem(runner, file, message)) + + @property + def failed(self) -> bool: + return bool(self.problems) + + def runners_with_errors(self) -> list[str]: + names: list[str] = [] + for problem in self.problems: + if problem.runner not in names: + names.append(problem.runner) + return names + + +def run_git(root: Path, *args: str) -> bytes: + result = subprocess.run( + ["git", "-C", str(root), *args], + check=True, + stdout=subprocess.PIPE, + stderr=subprocess.PIPE, + ) + return result.stdout + + +def git_tracked(root: Path) -> dict[str, str]: + """Map every tracked path under runners/ to its git mode. + + The git index rather than the filesystem, so that gitignored local junk + (the maintainer has untracked .DS_Store files) never produces a finding, + and so that what is validated is exactly what is committed. The mode comes + along for free and reveals symlinks (120000) and exec bits (100755). + """ + output = run_git(root, "ls-files", "-s", "-z", "--", "runners") + tracked = {} + for record in output.split(b"\0"): + if not record: + continue + info, _, path = record.partition(b"\t") + mode = info.split(b" ", 1)[0].decode("ascii") + tracked[path.decode("utf-8", "surrogateescape")] = mode + return tracked + + +def changed_runners(root: Path, base_sha: str) -> set[str]: + output = run_git(root, "diff", "--name-only", "-z", base_sha, "HEAD") + names = set() + for raw in output.split(b"\0"): + if not raw: + continue + path = raw.decode("utf-8", "surrogateescape") + parts = path.split("/") + if len(parts) >= 3 and parts[0] == "runners": + names.add(parts[1]) + return names + + +def runner_names_on_disk(tracked: dict[str, str]) -> list[str]: + names = set() + for path in tracked: + parts = path.split("/") + if len(parts) >= 3 and parts[0] == "runners": + names.add(parts[1]) + return sorted(names) + + +def check_entry_name(raw: str) -> str | None: + if "\x00" in raw: + return "contains a NUL byte" + if not raw.isascii(): + return "contains non-ASCII characters" + if "\\" in raw: + return "contains a backslash (a Windows-style path separator)" + if raw.startswith("/"): + return "is an absolute path" + if len(raw) > 1 and raw[1] == ":": + return "contains a drive letter" + parts = raw.rstrip("/").split("/") + if any(part == ".." for part in parts): + return "escapes the archive root (`..`)" + if any(part in ("", ".") for part in parts): + return "contains an empty or `.` path component" + if len(raw) > MAX_ENTRY_NAME_LENGTH: + return "is unreasonably long" + return None + + +def check_zip_container(data: bytes, runner: str, path: str, report: Report) -> bool: + index = data.rfind(EOCD_SIGNATURE) + if index < 0 or len(data) - index < EOCD_SIZE: + report.error( + runner, path, "Not a valid zip archive (no end-of-central-directory record)." + ) + return False + + cd_size, cd_offset = struct.unpack_from(" bool: + ok = True + name = entry.filename + + if entry.flag_bits & 0x1 or entry.flag_bits & 0x40: + report.error(runner, path, f"`{name}` is encrypted. Runner archives must not be password-protected.") + ok = False + + if entry.compress_type not in (zipfile.ZIP_STORED, zipfile.ZIP_DEFLATED): + report.error( + runner, + path, + f"`{name}` uses compression method {entry.compress_type}; only stored and deflate are accepted.", + ) + ok = False + + is_directory = name.endswith("/") + mode = entry.external_attr >> 16 + + if mode: + # A zero format field means the writer only recorded permission bits + # (Python's zipfile.writestr does exactly this). Extractors treat that + # as a regular file, so only reject a format that is present and wrong. + file_format = mode & ZIP_FORMAT_MASK + if file_format and file_format not in (ZIP_REGULAR_FILE, ZIP_DIRECTORY): + kind = "symbolic link" if file_format == ZIP_SYMLINK else "special file" + report.error(runner, path, f"`{name}` is a {kind}. Only regular PNG files are allowed.") + ok = False + if mode & 0o7000: + report.error(runner, path, f"`{name}` has setuid, setgid, or sticky bits set.") + ok = False + if not is_directory and mode & 0o111: + report.error(runner, path, f"`{name}` has the executable bit set. Image files must not be executable.") + ok = False + else: + dos_attributes = entry.external_attr & 0xFF + if dos_attributes & DOS_HIDDEN: + report.error(runner, path, f"`{name}` is marked hidden.") + ok = False + if dos_attributes & DOS_SYSTEM: + report.error(runner, path, f"`{name}` is marked as a system file.") + ok = False + + return ok + + +def walk_png_chunks(data: bytes) -> list[tuple[str, int, bytes]]: + if not data.startswith(PNG_SIGNATURE): + raise PngError("it does not start with the PNG signature (this is not a PNG file)") + + offset = len(PNG_SIGNATURE) + chunks: list[tuple[str, int, bytes]] = [] + seen_iend = False + + while offset < len(data): + if seen_iend: + raise PngError( + f"{len(data) - offset} bytes of data follow the IEND chunk; a PNG must end at IEND" + ) + if len(data) - offset < 12: + raise PngError("a chunk header is truncated") + + (length,) = struct.unpack_from(">I", data, offset) + chunk_type = data[offset + 4 : offset + 8] + if not chunk_type.isalpha(): + raise PngError("a chunk type is not four ASCII letters") + if length > MAX_PNG_BYTES: + raise PngError(f"chunk {chunk_type.decode()} declares an implausible length ({length} bytes)") + + end = offset + 12 + length + if end > len(data): + raise PngError(f"chunk {chunk_type.decode()} runs past the end of the file") + + payload = data[offset + 8 : offset + 8 + length] + (expected_crc,) = struct.unpack_from(">I", data, offset + 8 + length) + if binascii.crc32(chunk_type + payload) & 0xFFFFFFFF != expected_crc: + raise PngError( + f"chunk {chunk_type.decode()} has a CRC mismatch (the file is corrupt or was tampered with)" + ) + + chunks.append((chunk_type.decode("ascii"), length, payload)) + if chunk_type == b"IEND": + seen_iend = True + offset = end + + if not seen_iend: + raise PngError("it has no IEND chunk (the file is truncated)") + if chunks[0][0] != "IHDR": + raise PngError("its first chunk is not IHDR") + return chunks + + +def check_png( + data: bytes, + *, + label: str, + path: str, + runner: str | None, + report: Report, + require_apng: bool, +) -> tuple[int, int] | None: + if len(data) > MAX_PNG_BYTES: + report.error( + runner, + path, + f"{label} is {len(data)} bytes, far larger than a 36px-tall image should ever be.", + ) + return None + + try: + chunks = walk_png_chunks(data) + except PngError as error: + report.error(runner, path, f"{label} is not a well-formed PNG: {error}.") + return None + + types = [chunk_type for chunk_type, _, _ in chunks] + header = chunks[0][2] + if len(header) < 13: + report.error(runner, path, f"{label} has a truncated IHDR chunk.") + return None + width, height, _, _, _, _, interlace = struct.unpack_from(">IIBBBBB", header, 0) + + # Adam7 pointlessly interlaces an image this small, and Pillow cannot decode + # the frames of an interlaced APNG at all, so it would also cost us the + # full-decode check that proves the file is really an image. The IHDR is + # CRC-verified, so the size is still worth reporting against. + if interlace: + report.error( + runner, + path, + f"{label} uses Adam7 interlacing, which is not supported. Re-export it without " + 'interlacing (in most tools this is the default, or an "interlaced" checkbox to ' + "leave unticked).", + ) + return width, height + + for chunk_type, length, _ in chunks: + if chunk_type in ("tEXt", "zTXt", "iTXt") and length > MAX_TEXT_CHUNK_BYTES: + report.error( + runner, + path, + f"{label} has a {length}-byte `{chunk_type}` metadata chunk, which is implausibly " + "large for a runner image and a common place to hide a payload. Strip the metadata " + "and export it again.", + ) + + if require_apng: + if "acTL" not in types: + report.error( + runner, + path, + f"{label} is a static PNG, not an APNG (it has no `acTL` chunk), so it will not " + "animate in the gallery.", + ) + elif "IDAT" in types and types.index("acTL") > types.index("IDAT"): + report.error(runner, path, f"{label} has its `acTL` chunk after `IDAT`, which is not a valid APNG.") + + # Force every pixel of every frame through zlib and libpng. A file that + # merely has valid chunk framing still has to survive this to be an image. + try: + with Image.open(io.BytesIO(data)) as image: + image.verify() + with Image.open(io.BytesIO(data)) as image: + image.load() + for index in range(getattr(image, "n_frames", 1)): + image.seek(index) + image.load() + except Exception as error: + report.error(runner, path, f"{label} could not be decoded as an image ({error}).") + return None + + return width, height + + +def classify_unexpected_entry(name: str, runner: str) -> str: + """The reason an entry is rejected, phrased so several entries sharing a + reason can be reported together. Contributors hit these in batches — a zip + carries one `._x` per frame — so one message per reason beats one per file. + """ + base = name.rstrip("/").split("/")[-1] + top = name.split("/")[0] + for junk, reason in JUNK_REASONS.items(): + if base == junk or top == junk: + return reason.format(name=runner) + if base.startswith("._"): + return APPLEDOUBLE_REASON.format(name=runner) + if base.startswith("."): + return HIDDEN_REASON + return f"not allowed here. The archive must contain only `{runner}-frames/{runner}-frame-N.png` files." + + +def report_unexpected_entries(runner: str, path: str, grouped: dict[str, list[str]], report: Report) -> None: + for reason, names in grouped.items(): + shown = ", ".join(f"`{entry}`" for entry in names[:MAX_REPORTED_UNEXPECTED]) + remainder = len(names) - MAX_REPORTED_UNEXPECTED + if remainder > 0: + shown += f" and {remainder} more" + verb = "is" if len(names) == 1 else "are" + report.error(runner, path, f"{shown} {verb} {reason}") + + +def validate_zip(root: Path, name: str, report: Report) -> None: + path = f"runners/{name}/{name}-frames.zip" + data = (root / path).read_bytes() + + if len(data) > MAX_ZIP_BYTES: + report.error( + name, + path, + f"The zip is {len(data)} bytes. Runner frame archives are a few tens of kilobytes; " + "this is implausibly large.", + ) + return + + if not check_zip_container(data, name, path, report): + return + + try: + archive = zipfile.ZipFile(io.BytesIO(data)) + except zipfile.BadZipFile as error: + report.error(name, path, f"The zip could not be opened: {error}.") + return + + entries = archive.infolist() + if len(entries) > MAX_ENTRIES: + report.error(name, path, f"The zip has {len(entries)} entries; at most {MAX_ENTRIES} are allowed.") + return + + root_directory = f"{name}-frames/" + frame_re = re.compile(rf"^{re.escape(name)}-frame-(\d+)\.png$") + + # Reported as one message rather than as an unexpected-entry error per file, + # which for a 30-frame runner would bury the actual problem. + if not any(entry.filename.startswith(root_directory) for entry in entries): + report.error( + name, + path, + f"The zip has no `{root_directory}` directory; its {len(entries)} entries sit at the " + f"root of the archive. Zip the `{root_directory}` directory itself, not its contents: " + + REZIP_COMMAND.format(name=name), + ) + return + + frames: dict[int, tuple[str, bytes]] = {} + total_uncompressed = 0 + top_levels: set[str] = set() + unexpected: dict[str, list[str]] = {} + entries_ok = True + + for entry in entries: + entry_name = entry.filename + + name_problem = check_entry_name(entry_name) + if name_problem is not None: + report.error(name, path, f"The zip entry `{entry_name}` {name_problem}.") + entries_ok = False + continue + + if not check_entry_metadata(entry, name, path, report): + entries_ok = False + continue + + top_levels.add(entry_name.split("/")[0]) + + if entry_name == root_directory: + continue + + base = entry_name[len(root_directory) :] if entry_name.startswith(root_directory) else None + match = frame_re.match(base) if base else None + + if match is None: + unexpected.setdefault(classify_unexpected_entry(entry_name, name), []).append(entry_name) + entries_ok = False + continue + + if entry.file_size > MAX_ENTRY_UNCOMPRESSED: + report.error(name, path, f"`{entry_name}` expands to {entry.file_size} bytes, which is implausibly large.") + entries_ok = False + continue + + ratio = entry.file_size / max(entry.compress_size, 1) + if ratio > MAX_RATIO: + report.error( + name, + path, + f"`{entry_name}` has a compression ratio of {ratio:.0f}:1, which suggests a zip bomb.", + ) + entries_ok = False + continue + + try: + blob = archive.read(entry) + except Exception as error: + report.error(name, path, f"`{entry_name}` could not be extracted: {error}.") + entries_ok = False + continue + + # The local header can lie about file_size; this is what actually caps a bomb. + if len(blob) != entry.file_size: + report.error( + name, + path, + f"`{entry_name}` expanded to {len(blob)} bytes but the zip header declares " + f"{entry.file_size}. The archive is malformed.", + ) + entries_ok = False + continue + + total_uncompressed += len(blob) + if total_uncompressed > MAX_TOTAL_UNCOMPRESSED: + report.error(name, path, "The zip expands to more than 8 MB in total, which suggests a zip bomb.") + return + + index = int(match.group(1)) + if index in frames: + report.error( + name, + path, + f"`{entry_name}` and `{root_directory}{frames[index][0]}` are both frame {index}.", + ) + entries_ok = False + continue + frames[index] = (base, blob) + + report_unexpected_entries(name, path, unexpected, report) + + if len(top_levels) > 1: + listed = sorted(top_levels)[:MAX_REPORTED_UNEXPECTED] + suffix = ", ..." if len(top_levels) > MAX_REPORTED_UNEXPECTED else "" + report.error( + name, + path, + f"The zip has {len(top_levels)} top-level entries (" + + ", ".join(f"`{top}`" for top in listed) + + suffix + + f"). It must contain a single `{root_directory}` directory.", + ) + return + + if not entries_ok: + return + + if len(frames) < MIN_FRAMES: + report.error( + name, + path, + f"The zip contains {len(frames)} frame(s). A runner animation needs at least {MIN_FRAMES}.", + ) + return + + expected_indices = set(range(len(frames))) + if set(frames) != expected_indices: + missing = sorted(expected_indices - set(frames)) + extra = sorted(set(frames) - expected_indices) + details = [] + if missing: + details.append("missing " + ", ".join(f"`{name}-frame-{i}.png`" for i in missing)) + if extra: + details.append("unexpected " + ", ".join(f"`{name}-frame-{i}.png`" for i in extra)) + report.error( + name, + path, + f"Frames must be numbered consecutively from 0 to {len(frames) - 1}, but the archive has " + + " and ".join(details) + + ".", + ) + return + + sizes: dict[tuple[int, int], str] = {} + for index in sorted(frames): + base, blob = frames[index] + size = check_png( + blob, + label=f"Frame `{base}`", + path=path, + runner=name, + report=report, + require_apng=False, + ) + if size is None: + continue + width, height = size + if height != FRAME_HEIGHT: + report.error( + name, + path, + f"Frame `{base}` is {width}x{height}. Every frame must be exactly {FRAME_HEIGHT}px tall.", + ) + if not MIN_WIDTH <= width <= MAX_WIDTH: + report.error( + name, + path, + f"Frame `{base}` is {width}px wide. Frames must be between {MIN_WIDTH}px and {MAX_WIDTH}px wide.", + ) + sizes.setdefault(size, base) + + if len(sizes) > 1: + listed = ", ".join(f"{w}x{h} (e.g. `{base}`)" for (w, h), base in sorted(sizes.items())) + report.error(name, path, f"Frames have different sizes: {listed}. All frames must share one size.") + + +def validate_metadata(root: Path, name: str, report: Report) -> None: + path = f"runners/{name}/metadata.json" + raw = (root / path).read_bytes() + + if len(raw) > MAX_JSON_BYTES: + report.error(name, path, f"The file is {len(raw)} bytes; `metadata.json` should be a few lines of JSON.") + return + + try: + text = raw.decode("utf-8") + except UnicodeDecodeError as error: + report.error(name, path, f"The file is not valid UTF-8: {error}.") + return + + try: + metadata = json.loads(text) + except json.JSONDecodeError as error: + report.error(name, path, f"The file is not valid JSON: line {error.lineno} column {error.colno}: {error.msg}.") + return + + if not isinstance(metadata, dict): + report.error(name, path, "The top level must be a JSON object.") + return + + keys = set(metadata) + missing = METADATA_KEYS - keys + unexpected = keys - METADATA_KEYS + if missing: + report.error(name, path, "Missing required key(s): " + ", ".join(f"`{key}`" for key in sorted(missing)) + ".") + if unexpected: + report.error( + name, + path, + "Unexpected key(s): " + + ", ".join(f"`{key}`" for key in sorted(unexpected)) + + ". Only `author` and `displayName` are used.", + ) + + for key in sorted(METADATA_KEYS & keys): + value = metadata[key] + if not isinstance(value, str): + report.error(name, path, f"`{key}` must be a string.") + continue + if not value.strip(): + report.error(name, path, f"`{key}` must not be empty.") + continue + if len(value) > MAX_METADATA_VALUE_LENGTH: + report.error(name, path, f"`{key}` is {len(value)} characters; at most {MAX_METADATA_VALUE_LENGTH} are allowed.") + if any(ord(character) < 0x20 or ord(character) == 0x7F for character in value): + report.error(name, path, f"`{key}` contains control characters.") + + +def validate_preview(root: Path, name: str, report: Report) -> None: + path = f"runners/{name}/preview.png" + data = (root / path).read_bytes() + size = check_png( + data, + label="`preview.png`", + path=path, + runner=name, + report=report, + require_apng=True, + ) + if size is None: + return + width, height = size + if height != FRAME_HEIGHT: + report.error(name, path, f"`preview.png` is {width}x{height}. It must be exactly {FRAME_HEIGHT}px tall.") + if width > MAX_WIDTH: + report.error(name, path, f"`preview.png` is {width}px wide. It must be at most {MAX_WIDTH}px wide.") + + +def validate_directory_contents(name: str, tracked: dict[str, str], report: Report) -> set[str]: + prefix = f"runners/{name}/" + present: set[str] = set() + directory_path = f"runners/{name}" + + for path, mode in tracked.items(): + if not path.startswith(prefix): + continue + relative = path[len(prefix) :] + if "/" in relative: + report.error( + name, + path, + f"`{relative}` is inside a subdirectory. A runner directory holds exactly three files, flat.", + ) + continue + present.add(relative) + if mode == "120000": + report.error(name, path, f"`{relative}` is committed as a symbolic link. It must be a regular file.") + elif mode == "100755": + report.error(name, path, f"`{relative}` is committed with the executable bit set. Run `chmod 644` on it.") + elif mode != "100644": + report.error(name, path, f"`{relative}` has an unexpected git mode ({mode}). It must be a regular file.") + + expected = {template.format(name=name) for template in EXPECTED_FILES} + missing = expected - present + unexpected = present - expected + + if "preview.gif" in unexpected and "preview.png" in missing: + report.error( + name, + f"{prefix}preview.gif", + "The gallery renders `preview.png` and does not support GIF. Convert your animation to an " + "animated PNG named `preview.png`, for example with " + "`ffmpeg -i preview.gif -plays 0 -f apng preview.png`.", + ) + missing.discard("preview.png") + unexpected.discard("preview.gif") + + for filename in sorted(missing): + report.error(name, f"{prefix}{filename}", f"`{filename}` is missing. " + describe_expected(name, filename)) + for filename in sorted(unexpected): + report.error( + name, + f"{prefix}{filename}", + f"`{filename}` does not belong here. A runner directory contains exactly " + f"`{name}-frames.zip`, `metadata.json`, and `preview.png`.", + ) + + if not present: + report.error(name, directory_path, "The runner directory is empty.") + + return present + + +def describe_expected(name: str, filename: str) -> str: + if filename == "metadata.json": + return 'It holds the runner\'s `author` and `displayName`, e.g. `{"author": "Your Name (YourGitHubID)", "displayName": "Display Name"}`.' + if filename == "preview.png": + return "Every runner needs an animated PNG preview, 36px tall and at most 100px wide." + return f"It holds the animation frames as `{name}-frames/{name}-frame-N.png`." + + +def validate_runner(root: Path, name: str, tracked: dict[str, str], report: Report) -> None: + report.checked.append(name) + + if not RUNNER_NAME_RE.match(name): + report.error( + name, + f"runners/{name}", + f"`{name}` is not a valid runner name. Use lowercase letters, digits, and hyphens, " + "e.g. `welsh-corgi`.", + ) + return + + present = validate_directory_contents(name, tracked, report) + + if f"{name}-frames.zip" in present: + validate_zip(root, name, report) + if "metadata.json" in present: + validate_metadata(root, name, report) + if "preview.png" in present: + validate_preview(root, name, report) + + +def validate_manifest(root: Path, tracked: dict[str, str], report: Report) -> None: + """Always checked in full, for every run. + + Reading the manifest and listing directories costs nothing (no archive is + opened), and manifest/directory correspondence is inherently global: a PR + that reorders the list can drop an entry for a runner it never touched. + A name listed with a broken directory is silently dropped from the gallery + by script.js, which is the failure this whole check exists to catch. + """ + path = "runners/manifest.json" + if path not in tracked: + report.error(None, path, "`runners/manifest.json` is missing.") + return + + raw = (root / path).read_bytes() + if len(raw) > MAX_JSON_BYTES: + report.error(None, path, f"The file is {len(raw)} bytes, which is far larger than expected.") + return + + try: + manifest = json.loads(raw.decode("utf-8")) + except (UnicodeDecodeError, json.JSONDecodeError) as error: + report.error(None, path, f"The file is not valid JSON: {error}.") + return + + if not isinstance(manifest, dict) or set(manifest) != {"runners"}: + report.error(None, path, 'The file must be a JSON object with exactly one key, `runners`.') + return + + listed = manifest["runners"] + if not isinstance(listed, list) or not all(isinstance(item, str) for item in listed): + report.error(None, path, "`runners` must be a list of strings.") + return + + seen: set[str] = set() + for item in listed: + if not RUNNER_NAME_RE.match(item): + report.error( + None, + path, + f"`{item}` is not a valid runner name. Use lowercase letters, digits, and hyphens.", + ) + if item in seen: + report.error(None, path, f"`{item}` is listed more than once.") + seen.add(item) + + on_disk = set(runner_names_on_disk(tracked)) + + for item in sorted(seen - on_disk): + report.error( + None, + path, + f"`{item}` is listed in the manifest but `runners/{item}/` does not exist. The gallery " + "would silently skip it.", + ) + for item in sorted(on_disk - seen): + report.error( + None, + path, + f"`runners/{item}/` exists but `{item}` is not listed in the manifest, so it will not " + "appear in the gallery.", + ) + + +def validate(root: Path, targets: set[str] | None) -> Report: + report = Report() + tracked = git_tracked(root) + + validate_manifest(root, tracked, report) + + names = runner_names_on_disk(tracked) + if targets is not None: + names = [name for name in names if name in targets] + + for name in names: + try: + validate_runner(root, name, tracked, report) + except Exception as error: + report.internal_error = True + report.error( + name, + f"runners/{name}", + f"Internal error while validating this runner: {error!r}. This is a CI problem, " + "not a problem with your pull request.", + ) + + return report + + +def escape_data(value: str) -> str: + return value.replace("%", "%25").replace("\r", "%0D").replace("\n", "%0A") + + +def escape_property(value: str) -> str: + return escape_data(value).replace(":", "%3A").replace(",", "%2C") + + +def emit_annotations(report: Report) -> None: + for problem in report.problems: + title = f"Runner: {problem.runner}" if problem.runner else "Runner validation" + print( + f"::error file={escape_property(problem.file)}," + f"title={escape_property(title)}::{escape_data(problem.message)}" + ) + + +def plural(count: int, singular: str, plural_form: str | None = None) -> str: + word = singular if count == 1 else (plural_form or singular + "s") + return f"{count} {word}" + + +def emit_step_summary(report: Report, out) -> None: + out.write("## Runner validation\n\n") + + if not report.problems: + passed = plural(len(report.checked), "runner") + out.write(f"✅ **{passed} passed** — archive contents, PNG integrity, and spec conformance.\n\n") + out.write("
What was checked\n\n") + out.write( + "- Each `*-frames.zip` contains only PNG frames under a single `-frames/` directory — " + "no symlinks, no executables, no encrypted or Finder-generated entries, and no data appended " + "to the archive.\n" + "- Every PNG was fully decoded and proven to consist entirely of valid PNG chunks with correct " + "CRCs, ending at `IEND` with no trailing bytes. There is nowhere in these files for an " + "executable payload to hide.\n" + "- Every frame is exactly 36px tall, 10–100px wide, and all frames in an archive share one size.\n" + "- `metadata.json`, `preview.png` (an APNG), and `manifest.json` match each runner directory.\n" + ) + out.write("\n
\n") + return + + failed = [name for name in report.runners_with_errors() if name] + out.write( + f"❌ **{plural(len(report.problems), 'problem')} in {plural(len(failed), 'runner')}** — " + f"{len(report.checked) - len(failed)} of {plural(len(report.checked), 'checked runner')} passed.\n\n" + ) + + grouped: dict[str | None, list[Problem]] = {} + for problem in report.problems: + grouped.setdefault(problem.runner, []).append(problem) + + for runner in sorted(grouped, key=lambda value: (value is not None, value or "")): + heading = f"`{runner}`" if runner else "Repository" + out.write(f"### {heading}\n\n") + out.write("| File | Problem |\n| --- | --- |\n") + for problem in grouped[runner]: + message = problem.message.replace("|", "\\|") + out.write(f"| `{problem.file}` | {message} |\n") + out.write("\n") + + out.write("---\n\n") + out.write("Each problem is also annotated on the relevant file in the **Files changed** tab.\n") + out.write( + "See [CONTRIBUTING.md](https://github.com/runcat-dev/RunnerGallery/blob/main/CONTRIBUTING.md) " + "for the full requirements.\n" + ) + out.write("You can run these checks yourself before pushing:\n\n") + out.write("```sh\npip install -r .github/scripts/requirements.txt\npython3 .github/scripts/validate_runners.py --all\n```\n") + + +def emit_human(report: Report) -> None: + if not report.problems: + print(f"OK: {plural(len(report.checked), 'runner')} passed: {', '.join(report.checked)}") + return + + grouped: dict[str | None, list[Problem]] = {} + for problem in report.problems: + grouped.setdefault(problem.runner, []).append(problem) + + for runner in sorted(grouped, key=lambda value: (value is not None, value or "")): + print(f"\n{runner or 'repository'}:") + for problem in grouped[runner]: + print(f" {problem.file}") + print(f" {problem.message}") + + print(f"\n{plural(len(report.problems), 'problem')} found.") + + +def main() -> int: + parser = argparse.ArgumentParser(description="Validate runner assets.") + group = parser.add_mutually_exclusive_group(required=True) + group.add_argument("--all", action="store_true", help="validate every runner") + group.add_argument("--changed-since", metavar="BASE_SHA", help="validate runners touched since BASE_SHA") + arguments = parser.parse_args() + + root = Path(__file__).resolve().parents[2] + + try: + targets = None if arguments.all else changed_runners(root, arguments.changed_since) + report = validate(root, targets) + except subprocess.CalledProcessError as error: + stderr = error.stderr.decode("utf-8", "replace").strip() + print(f"Failed to run git: {stderr}", file=sys.stderr) + return 2 + + in_actions = bool(os.environ.get("GITHUB_ACTIONS")) + if in_actions: + emit_annotations(report) + else: + emit_human(report) + + summary_path = os.environ.get("GITHUB_STEP_SUMMARY") + if summary_path: + with open(summary_path, "a", encoding="utf-8") as out: + emit_step_summary(report, out) + elif in_actions: + emit_step_summary(report, sys.stdout) + + if report.internal_error: + return 2 + return 1 if report.failed else 0 + + +if __name__ == "__main__": + sys.exit(main()) diff --git a/.github/workflows/validate-runners.yml b/.github/workflows/validate-runners.yml new file mode 100644 index 0000000..6558529 --- /dev/null +++ b/.github/workflows/validate-runners.yml @@ -0,0 +1,48 @@ +name: Validate runners + +# `pull_request`, never `pull_request_target`. This job feeds contributor-supplied +# binaries to zlib, libpng, and Pillow's C decoders; it is the last thing in this +# repository that should run with a writable token or access to secrets. A fork's +# `pull_request` run gets a read-only token and no secrets, and that sandbox is the +# actual security control here. +on: + pull_request: + types: [opened, synchronize, reopened] + workflow_dispatch: + +# Do NOT add a `paths:` filter. Once this is a required status check, a filtered-out +# pull request never reports the check and can never be merged. The job takes seconds. + +permissions: + contents: read + +concurrency: + group: validate-runners-${{ github.event.pull_request.number || github.sha }} + cancel-in-progress: true + +jobs: + validate: + name: Validate runner assets + runs-on: ubuntu-latest + timeout-minutes: 10 + steps: + - uses: actions/checkout@v4 + with: + fetch-depth: 0 + persist-credentials: false + + - uses: actions/setup-python@v5 + with: + python-version: '3.12' + + - run: pip install --disable-pip-version-check -r .github/scripts/requirements.txt + + - name: Validate + env: + BASE_SHA: ${{ github.event.pull_request.base.sha }} + run: | + if [ -n "${BASE_SHA:-}" ]; then + python3 .github/scripts/validate_runners.py --changed-since "$BASE_SHA" + else + python3 .github/scripts/validate_runners.py --all + fi From 27d7916360386edf35faa4af06d88c6801325a63 Mon Sep 17 00:00:00 2001 From: "Takuto NAKAMURA (Kyome)" Date: Wed, 15 Jul 2026 23:12:31 +0900 Subject: [PATCH 2/4] Rename Record Player frames to match the documented pattern Every other runner names its frames -frame-.png, but this archive used record-player-.png, so it was the one runner the new validation would reject. The frame images themselves are byte-for-byte unchanged; only the zip entry names differ. Co-Authored-By: Claude Opus 4.8 (1M context) --- .../record-player/record-player-frames.zip | Bin 9726 -> 9499 bytes 1 file changed, 0 insertions(+), 0 deletions(-) diff --git a/runners/record-player/record-player-frames.zip b/runners/record-player/record-player-frames.zip index cad43c62ff85a9d7e730855f5df9214103878e85..4ef6e32c19978fbca8e7f5562e48ab971375a880 100644 GIT binary patch delta 619 zcmez8J==>lz?+#xWOAOe_~duW;?iIq0|NsG14HBEn5I={a@VslFsuS%y~%Ht)dbRt z5_3~^P4o)#(ivB=h5p>QaUGku2w218hQ~3&`B$6Q0Tr)A(_jeJu#zq8=fsUFn;)`w z!Zh6gYAQ8(#8C{?kcXzp0IVsGZ6a9H8$Lgnrnf*%!PoX|+62_F7EO~8SkqdzAh4!A zqP;Lp-+-EKUF7+E6sX|IH`wvR;gkLeZr>pbK)AGmn}Lz#D}z3W+yI(UYVwF952zo6p=KDs&1lDN2FNL!=koc% z%y|nmC-~Z)O>2Q>fH2e?Be*&1@R$=L+6y!18_=9v7kNG(0Ga{9P;-po<{ZFdPL#|7 zHfA|yNPI|47FAYp4h><2L Date: Wed, 15 Jul 2026 23:12:31 +0900 Subject: [PATCH 3/4] Document frame naming and the local self-check The frame file name was only ever shown as an example, so state it as a requirement now that CI enforces it, and note that PNGs must not be interlaced. Also point contributors at the validator itself. Someone who runs it before pushing never has to see a red check at all, which matters more than any wording in the failure messages. Co-Authored-By: Claude Opus 4.8 (1M context) --- .github/pull_request_template.md | 3 ++- CONTRIBUTING.md | 21 +++++++++++++++++---- 2 files changed, 19 insertions(+), 5 deletions(-) diff --git a/.github/pull_request_template.md b/.github/pull_request_template.md index 40f8042..b17f959 100644 --- a/.github/pull_request_template.md +++ b/.github/pull_request_template.md @@ -12,7 +12,8 @@ ## Checklist for Adding a Runner -- [ ] All frames are PNG files with a height of exactly 36px, a width between 10px and 100px, and all frames share the same size. +- [ ] All frames are PNG files named `-frame-.png` (numbered from 0), with a height of exactly 36px, a width between 10px and 100px, and all frames share the same size. +- [ ] The zip contains only the frames — no `.DS_Store`, `__MACOSX/`, or `._*` entries. - [ ] The runner is my original work, or I hold the rights to it, and I consent to its distribution under this repository's license (Apache-2.0). - [ ] The runner does NOT contain third-party intellectual property (e.g., Pokémon, Mario, or other copyrighted/trademarked characters). - [ ] Files are placed at `runners//` as `-frames.zip`, `metadata.json`, and `preview.png`. diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 92b6bd8..57d96ec 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -34,12 +34,13 @@ A zip archive of a `-frames/` directory containing the animation fr Frame requirements: -- Format: PNG +- Format: PNG, not interlaced +- File name: `-frame-.png`, numbered consecutively from `0` - Height: exactly 36px - Width: between 10px and 100px - All frames must have the same size - A transparent background is recommended -- Avoid including Finder-generated files such as `.DS_Store`, `__MACOSX/`, and `._*` in the zip +- The zip must contain nothing but the frames — no Finder-generated files such as `.DS_Store`, `__MACOSX/`, or `._*` Tip for macOS users — creating the zip from the command line avoids Finder-generated junk: @@ -60,7 +61,7 @@ zip -r -frames.zip -frames -x "*.DS_Store" "__MACOSX/* An animated PNG (APNG) used for the gallery preview: -- Format: APNG (a static PNG will not animate in the gallery) +- Format: APNG, not interlaced (a static PNG will not animate in the gallery) - Height: 36px - Width: at most 100px @@ -77,7 +78,19 @@ Add your runner name to `runners/manifest.json`, following the existing ordering } ``` -### 4. Open a Pull Request +### 4. Check your runner locally + +Every Pull Request is automatically checked: the frame archive must contain nothing but your frames, and every image must be a genuine, fully decodable PNG. +You can run exactly the same checks yourself before pushing, so that you do not have to wait for CI: + +```sh +pip install -r .github/scripts/requirements.txt +python3 .github/scripts/validate_runners.py --all +``` + +Anything it reports tells you which file is at fault and what to change. + +### 5. Open a Pull Request 1. Fork this repository and create a branch. 2. Commit your runner files and the manifest change. From 01ef93c765048441642f6ad6a6a68d7a733c89b3 Mon Sep 17 00:00:00 2001 From: "Takuto NAKAMURA (Kyome)" Date: Wed, 15 Jul 2026 23:16:05 +0900 Subject: [PATCH 4/4] Report the validation result in the job log too The first run produced a completely empty step log: on success the validator emits no annotations, and the summary goes to a file. That leaves no way to tell from the log whether it validated a runner or silently validated nothing, which matters because the scope comes from a diff and an empty scope still exits 0. Print the result to stdout in both modes, and say so explicitly when nothing was in scope. Also move off Node 20, which the runner now warns about. Co-Authored-By: Claude Opus 4.8 (1M context) --- .github/scripts/validate_runners.py | 13 ++++++++++--- .github/workflows/validate-runners.yml | 4 ++-- 2 files changed, 12 insertions(+), 5 deletions(-) diff --git a/.github/scripts/validate_runners.py b/.github/scripts/validate_runners.py index 20c52d9..76923d7 100644 --- a/.github/scripts/validate_runners.py +++ b/.github/scripts/validate_runners.py @@ -900,6 +900,9 @@ def emit_step_summary(report: Report, out) -> None: out.write("## Runner validation\n\n") if not report.problems: + if not report.checked: + out.write("✅ No runner was changed by this pull request; only the manifest was checked.\n") + return passed = plural(len(report.checked), "runner") out.write(f"✅ **{passed} passed** — archive contents, PNG integrity, and spec conformance.\n\n") out.write("
What was checked\n\n") @@ -947,7 +950,10 @@ def emit_step_summary(report: Report, out) -> None: def emit_human(report: Report) -> None: if not report.problems: - print(f"OK: {plural(len(report.checked), 'runner')} passed: {', '.join(report.checked)}") + if report.checked: + print(f"OK: {plural(len(report.checked), 'runner')} passed: {', '.join(report.checked)}") + else: + print("No runner was changed; only the manifest was checked.") return grouped: dict[str | None, list[Problem]] = {} @@ -983,8 +989,9 @@ def main() -> int: in_actions = bool(os.environ.get("GITHUB_ACTIONS")) if in_actions: emit_annotations(report) - else: - emit_human(report) + # Always in the log too, so that a run which validated nothing at all reads + # as such instead of as a silent pass. + emit_human(report) summary_path = os.environ.get("GITHUB_STEP_SUMMARY") if summary_path: diff --git a/.github/workflows/validate-runners.yml b/.github/workflows/validate-runners.yml index 6558529..f73bded 100644 --- a/.github/workflows/validate-runners.yml +++ b/.github/workflows/validate-runners.yml @@ -26,12 +26,12 @@ jobs: runs-on: ubuntu-latest timeout-minutes: 10 steps: - - uses: actions/checkout@v4 + - uses: actions/checkout@v7 with: fetch-depth: 0 persist-credentials: false - - uses: actions/setup-python@v5 + - uses: actions/setup-python@v6 with: python-version: '3.12'