diff --git a/.github/workflows/ci-apply.yml b/.github/workflows/ci-apply.yml new file mode 100644 index 0000000..046e97c --- /dev/null +++ b/.github/workflows/ci-apply.yml @@ -0,0 +1,294 @@ +name: PR Apply + +# Trusted counterpart to ci-check.yml (PR Check). Runs with this repo's +# write-scoped GITHUB_TOKEN via `workflow_run`, AFTER PR Check has finished +# safely (no secrets, fork content fully executed there instead of here). +# +# TWO HARD RULES, both load-bearing. Breaking either one reintroduces the +# pwn-request hole this split exists to close: +# +# 1. NEVER execute, import, or `unittest discover` anything from the fork's +# checkout. This job only applies a text patch (`git apply`), commits, +# pushes, and runs auto_apply_version_metadata.py from a SEPARATE checkout +# of this repo's own main branch - never the fork's copy of it. +# +# 2. NEVER interpolate a `${{ }}` expression into a `run:` block unless the +# value is fixed and trusted. `${{ }}` is substituted textually BEFORE the +# shell parses the script, so surrounding quotes do NOT contain it - a PR +# branch named `a";id;"` becomes live shell. Pass values via `env:` and +# reference them as "$VAR", which the shell treats as data. +# +# 3. NEVER identify the target PR by commit sha alone. A sha is a value, not an +# identity: forks share object storage, so anyone can push ANOTHER PR's head +# commit onto a branch of their own, open a PR at it, and then close that PR +# mid-run so the commit -> PR lookup below resolves to the victim's PR - at +# which point this job would push the attacker's artifact to the victim's +# branch. The resolved PR must be pinned to workflow_run.head_repository AND +# head_branch AND head_sha, so a run can only ever write to its own branch. +# +# On trust: a fork PR fully controls ci-check.yml itself (GitHub runs the +# workflow file from the PR's own merge ref for `pull_request` events - that +# is why that job gets a read-only, secret-less token). So EVERYTHING in the +# pr-fixups artifact is attacker-authored. PR identity is therefore resolved +# from the workflow_run payload + the API, never from the artifact; the +# artifact supplies only the patch, which is allowlist-validated below and +# only ever lands on the fork's own branch. + +on: + workflow_run: + workflows: ["PR Check"] + types: [completed] + +permissions: + contents: write + pull-requests: write + actions: read # required to download an artifact from another workflow run + +concurrency: + group: pr-apply-${{ github.event.workflow_run.head_repository.full_name }}-${{ github.event.workflow_run.head_branch }} + cancel-in-progress: false + +jobs: + apply: + if: >- + github.event.workflow_run.conclusion == 'success' && + github.event.workflow_run.event == 'pull_request' + runs-on: ubuntu-latest + steps: + # Resolves which PR this run belongs to using ONLY trusted inputs: the + # workflow_run payload (set by GitHub, not forgeable by the PR author) + # and the REST API. workflow_run.pull_requests is empty for fork PRs, + # hence the commit -> PR association lookup. That lookup ANSWERS with a + # PR but does not PROVE it is this run's PR - a sha can be adopted by any + # fork even though it cannot be forged - so the result is pinned to the + # payload's head repo/branch/sha below. See HARD RULE 3 in the header. + - name: Resolve and validate PR (trusted sources only) + id: pr + env: + GH_TOKEN: ${{ secrets.GITHUB_TOKEN }} + REPO: ${{ github.repository }} + RUN_HEAD_SHA: ${{ github.event.workflow_run.head_sha }} + RUN_HEAD_REPO: ${{ github.event.workflow_run.head_repository.full_name }} + RUN_HEAD_BRANCH: ${{ github.event.workflow_run.head_branch }} + run: | + set -euo pipefail + + skip() { echo "$1"; echo "proceed=0" >> "$GITHUB_OUTPUT"; exit 0; } + + [[ "$RUN_HEAD_SHA" =~ ^[0-9a-f]{40}$ ]] || { + echo "Unexpected workflow_run head sha shape; refusing." >&2 + exit 1 + } + + gh api "repos/${REPO}/commits/${RUN_HEAD_SHA}/pulls" > pulls.json + [ "$(jq 'length' pulls.json)" = "1" ] \ + || skip "Not exactly one PR associated with ${RUN_HEAD_SHA}; skipping." + + PR_NUMBER="$(jq -r '.[0].number' pulls.json)" + [[ "$PR_NUMBER" =~ ^[0-9]+$ ]] || skip "Bad PR number; skipping." + + gh pr view "$PR_NUMBER" --repo "$REPO" \ + --json state,baseRefName,headRefName,headRefOid,maintainerCanModify,headRepository,headRepositoryOwner \ + > pr.json + + STATE="$(jq -r .state pr.json)" + BASE_REF="$(jq -r .baseRefName pr.json)" + HEAD_REF="$(jq -r .headRefName pr.json)" + HEAD_SHA="$(jq -r .headRefOid pr.json)" + CAN_MODIFY="$(jq -r .maintainerCanModify pr.json)" + FORK="$(jq -r '.headRepositoryOwner.login + "/" + .headRepository.name' pr.json)" + + # Branch/repo names are attacker-chosen strings, and git happily + # accepts refnames containing ` $( ) ; | ' " - so anything outside + # this conservative set is refused rather than carried forward. + # Bash =~ anchors to the whole string, so embedded newlines (which + # would otherwise inject extra $GITHUB_OUTPUT keys) are rejected too. + [[ "$HEAD_REF" =~ ^[A-Za-z0-9._/-]{1,255}$ ]] || skip "Unsafe branch name; skipping." + [[ "$FORK" =~ ^[A-Za-z0-9._-]+/[A-Za-z0-9._-]+$ ]] || skip "Unsafe repo name; skipping." + [[ "$HEAD_SHA" =~ ^[0-9a-f]{40}$ ]] || skip "Unsafe head sha; skipping." + + [ "$STATE" = "OPEN" ] || skip "PR not open; skipping." + [ "$BASE_REF" = "main" ] || skip "PR not targeting main; skipping." + + # HARD RULE 3: pin the resolved PR to the run that produced the + # artifact. The sha check alone is not enough - a sha is adoptable by + # any fork, so on its own it would let a run push to somebody else's + # PR branch. These are compared only against FORK/HEAD_REF, which the + # regexes above already validated, so no shape check is needed here: + # a null head_repository yields "" and simply fails to match, which is + # the fail-closed direction. Do not "simplify" these away. + [ "$FORK" = "$RUN_HEAD_REPO" ] \ + || skip "Resolved PR head repo != the run's head repo; skipping." + [ "$HEAD_REF" = "$RUN_HEAD_BRANCH" ] \ + || skip "Resolved PR head branch != the run's head branch; skipping." + [ "$HEAD_SHA" = "$RUN_HEAD_SHA" ] \ + || skip "PR head moved since PR Check ran; skipping." + + # maintainerCanModify only has meaning for cross-fork PRs; GitHub + # reports false for a PR opened from a branch in this same repo, + # where we can always push because we own the branch. + if [ "$FORK" != "$REPO" ] && [ "$CAN_MODIFY" != "true" ]; then + echo "pr_number=$PR_NUMBER" >> "$GITHUB_OUTPUT" + echo "needs_comment=1" >> "$GITHUB_OUTPUT" + skip "Maintainer edits disabled; cannot push." + fi + + { + echo "pr_number=$PR_NUMBER" + echo "head_ref=$HEAD_REF" + echo "head_sha=$HEAD_SHA" + echo "fork=$FORK" + echo "proceed=1" + } >> "$GITHUB_OUTPUT" + + - name: Comment if maintainer edits are disabled (actionable, not self-resolving) + if: steps.pr.outputs.needs_comment == '1' + env: + GH_TOKEN: ${{ secrets.GITHUB_TOKEN }} + REPO: ${{ github.repository }} + PR_NUMBER: ${{ steps.pr.outputs.pr_number }} + run: | + set -euo pipefail + gh pr comment "$PR_NUMBER" --repo "$REPO" \ + --body "I can't push the automatic formatting/metadata commits to this PR because \"Allow edits from maintainers\" is disabled. Please enable it, or apply \`autopep8\` and \`test/auto_apply_*_metadata.py\` locally." + + - name: Download PR fixups artifact (untrusted data) + if: steps.pr.outputs.proceed == '1' + uses: actions/download-artifact@v8 + with: + name: pr-fixups + path: pr-data + github-token: ${{ secrets.GITHUB_TOKEN }} + run-id: ${{ github.event.workflow_run.id }} + + # Checking out fork PR content is what actions/checkout >=v6 refuses by + # default; the opt-in is reviewed and intentional here because nothing + # below ever EXECUTES this checkout - only git plumbing touches it. + # persist-credentials:false keeps the write token out of the untrusted + # working tree's .git/config; the push below authenticates explicitly. + - name: Checkout PR branch (fork) - allow-unsafe-pr-checkout is reviewed & intentional + if: steps.pr.outputs.proceed == '1' + uses: actions/checkout@v7 + with: + repository: ${{ steps.pr.outputs.fork }} + ref: ${{ steps.pr.outputs.head_sha }} + token: ${{ secrets.GITHUB_TOKEN }} + fetch-depth: 0 + persist-credentials: false + allow-unsafe-pr-checkout: true + path: pr + + - name: Checkout TRUSTED scripts from our own main branch + if: steps.pr.outputs.proceed == '1' + uses: actions/checkout@v7 + with: + ref: main + sparse-checkout: | + test/auto_apply_version_metadata.py + sparse-checkout-cone-mode: false + persist-credentials: false + path: trusted + + - name: Re-verify checked-out head matches the resolved head + if: steps.pr.outputs.proceed == '1' + working-directory: pr + env: + HEAD_SHA: ${{ steps.pr.outputs.head_sha }} + run: | + set -euo pipefail + test "$(git rev-parse HEAD)" = "$HEAD_SHA" + + - name: Validate patch (allowlisted paths only, no symlinks/binaries) + if: steps.pr.outputs.proceed == '1' + working-directory: pr + run: | + set -euo pipefail + PATCH="${GITHUB_WORKSPACE}/pr-data/fixups.patch" + if [ ! -s "$PATCH" ]; then + echo "Empty patch, nothing to validate" + exit 0 + fi + if grep -qE '^(deleted file mode 120000|new mode 120000|new file mode 120000|Binary files)' "$PATCH"; then + echo "Patch contains symlinks or binary content - refusing" >&2 + exit 1 + fi + # Deliberately excludes .github/** and test/** - a "[ci]"-authored + # commit touching CI config or the test suite is exactly what a + # reviewer would wave through, so those fail closed and are left to + # the contributor to format locally. + ALLOW='^(plugins/(minigames|utilities|maps)/[^/]+\.py|plugin_manager\.py|index\.json|plugins/(minigames|utilities|maps)\.json|CHANGELOG\.md)$' + # Redirect from a file rather than piping into `while`: a pipeline + # would run the loop in a subshell, where `exit 1` would not reliably + # fail the step. + git apply --numstat "$PATCH" | cut -f3 > "${RUNNER_TEMP}/patch_paths.txt" + while IFS= read -r f; do + [ -n "$f" ] || continue + if [[ ! "$f" =~ $ALLOW ]]; then + echo "Patch touches disallowed path: $f" >&2 + exit 1 + fi + done < "${RUNNER_TEMP}/patch_paths.txt" + git apply --check "$PATCH" + + - name: Apply fixups patch and commit + if: steps.pr.outputs.proceed == '1' + working-directory: pr + env: + GH_TOKEN: ${{ secrets.GITHUB_TOKEN }} + HEAD_REF: ${{ steps.pr.outputs.head_ref }} + FORK: ${{ steps.pr.outputs.fork }} + run: | + set -euo pipefail + PATCH="${GITHUB_WORKSPACE}/pr-data/fixups.patch" + git config user.name "github-actions[bot]" + git config user.email "41898282+github-actions[bot]@users.noreply.github.com" + if [ ! -s "$PATCH" ]; then + echo "Empty patch, nothing to apply" + exit 0 + fi + git apply "$PATCH" + # Stage exactly the paths the validation step allow-listed, so the + # commit scope can never exceed the scope that was validated. A + # broader pathspec (e.g. all of plugins/) would silently widen the + # blast radius if the allowlist ever regressed. + PATHS="${RUNNER_TEMP}/patch_paths.txt" + test -s "$PATHS" || { + echo "Validated path list missing or empty - refusing to stage" >&2 + exit 1 + } + git add -A --pathspec-from-file="$PATHS" + if ! git diff --cached --quiet; then + git commit -m "[ci] apply-plugin-metadata-and-formatting" + git push "https://x-access-token:${GH_TOKEN}@github.com/${FORK}.git" "HEAD:${HEAD_REF}" + fi + + - name: Apply Version Metadata using the TRUSTED script only (never the fork's copy) + if: steps.pr.outputs.proceed == '1' + working-directory: pr + env: + GH_TOKEN: ${{ secrets.GITHUB_TOKEN }} + HEAD_REF: ${{ steps.pr.outputs.head_ref }} + FORK: ${{ steps.pr.outputs.fork }} + run: | + set -euo pipefail + python "${GITHUB_WORKSPACE}/trusted/test/auto_apply_version_metadata.py" "$(git rev-parse HEAD)" + # That script only ever writes index.json and the category manifests + # (it opens every .py read-only), so stage precisely those rather + # than the whole plugins/ tree. + git add -A -- index.json \ + plugins/minigames.json plugins/utilities.json plugins/maps.json + if ! git diff --cached --quiet; then + git commit -m "[ci] apply-version-metadata" + git push "https://x-access-token:${GH_TOKEN}@github.com/${FORK}.git" "HEAD:${HEAD_REF}" + fi + + - name: On mechanical failure, notify the contributor + if: failure() && steps.pr.outputs.pr_number != '' + env: + GH_TOKEN: ${{ secrets.GITHUB_TOKEN }} + REPO: ${{ github.repository }} + PR_NUMBER: ${{ steps.pr.outputs.pr_number }} + run: | + set -euo pipefail + gh pr comment "$PR_NUMBER" --repo "$REPO" \ + --body "Automatic formatting/metadata could not be applied to this PR (patch touched a disallowed path, or a conflict occurred). A maintainer will need to look at this manually." diff --git a/.github/workflows/ci-check.yml b/.github/workflows/ci-check.yml new file mode 100644 index 0000000..ea4b8d8 --- /dev/null +++ b/.github/workflows/ci-check.yml @@ -0,0 +1,119 @@ +name: PR Check + +# Runs entirely inside GitHub's hardcoded read-only, no-secrets sandbox for +# `pull_request` events from forks. Fully executes fork-supplied code +# (autopep8, metadata scripts, unittest discover) - that's safe here ONLY +# because this token can't push anywhere and has no secrets. +# +# This workflow NEVER pushes anywhere. It only uploads a plain-text patch as +# a build artifact for ci-apply.yml (a separate, trusted workflow) to apply. +# +# Note that a fork PR can modify THIS FILE and have its version run (GitHub +# uses the workflow from the PR's own merge ref for `pull_request`). So +# nothing produced here is trustworthy, and ci-apply.yml is written on that +# assumption: it resolves PR identity from the workflow_run payload and the +# API, and treats the uploaded patch as untrusted input to be validated. +# +# DO NOT switch this to `pull_request_target` and do not add `permissions: +# contents: write` here "to save a round trip" - that reintroduces the +# pwn-request hole this split exists to close. + +on: + pull_request: + +permissions: + contents: read + +concurrency: + group: pr-check-${{ github.event.pull_request.number }} + cancel-in-progress: true + +jobs: + build: + runs-on: ubuntu-latest + steps: + # Credentials are deliberately persisted here (unlike ci-apply.yml): the + # `git fetch` below needs them on a private repo, and the workflow-level + # `permissions: contents: read` caps this token to read-only in every + # case - fork PRs get a read-only token from GitHub regardless. + - name: Checkout PR head + uses: actions/checkout@v7 + with: + ref: refs/pull/${{ github.event.pull_request.number }}/head + fetch-depth: 0 + + - name: Fetch PR base commit (for diffing) + env: + BASE_SHA: ${{ github.event.pull_request.base.sha }} + run: git fetch --no-tags --depth=1 origin "$BASE_SHA" + + - name: Set up Python + uses: actions/setup-python@v7 + with: + python-version: "3.12" + + - name: Install Dependencies + run: | + python -m pip install -U pip + python -m pip install -U pycodestyle==2.12.1 autopep8 + python -m pip install -U -r test/pip_reqs.txt + + # Filenames are attacker-controlled, so they are kept in files and + # passed as shell variables - never interpolated into a script or into + # $GITHUB_OUTPUT (where a file named after the heredoc delimiter, or one + # containing a newline, could inject extra output keys). + - name: Compute changed files + env: + BASE_SHA: ${{ github.event.pull_request.base.sha }} + run: | + set -euo pipefail + git diff --name-only "$BASE_SHA" HEAD > "${RUNNER_TEMP}/changed_files.txt" + git diff --name-only -z "$BASE_SHA" HEAD -- '*.py' > "${RUNNER_TEMP}/changed_py.z" + cat "${RUNNER_TEMP}/changed_files.txt" + + - name: Apply AutoPEP8 (changed .py files only) + run: | + set -euo pipefail + if [ -s "${RUNNER_TEMP}/changed_py.z" ]; then + xargs -0 -r autopep8 --in-place --max-line-length=100 \ + < "${RUNNER_TEMP}/changed_py.z" + fi + + # PLUGMAN_BASE_REF tells the script which tree counts as "already + # published". Without it the script compares against the PR's own + # manifest, which ci-apply.yml has already written the new version into - + # so every re-run of this workflow (including the one ci-apply.yml's push + # triggers) would fail with "Version cant be lower or equal". + - name: Apply Plugin Metadata (writes null version placeholders) + env: + PLUGMAN_BASE_REF: ${{ github.event.pull_request.base.sha }} + run: | + set -euo pipefail + python test/auto_apply_plugin_metadata.py "$(cat "${RUNNER_TEMP}/changed_files.txt")" + + # IMPORTANT: capture the patch BEFORE the version-metadata preview step + # below. commit_sha values computed there are fictitious (no real + # commit exists yet) and must never be part of what ci-apply.yml applies. + - name: Snapshot fixups patch + run: | + set -euo pipefail + mkdir -p "${RUNNER_TEMP}/pr-data" + git diff > "${RUNNER_TEMP}/pr-data/fixups.patch" + wc -l "${RUNNER_TEMP}/pr-data/fixups.patch" + + - name: Apply Version Metadata (local preview only - not uploaded) + run: | + python test/auto_apply_version_metadata.py "$(git rev-parse HEAD)" + + - name: "Execute Tests (lenient: new-entry history checks deferred to ci-apply/push-to-main)" + env: + PLUGMAN_CI_LENIENT_HISTORY: "1" + run: | + python -m unittest discover -v + + - name: Upload patch for ci-apply.yml + uses: actions/upload-artifact@v7 + with: + name: pr-fixups + path: ${{ runner.temp }}/pr-data + retention-days: 5 diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 15fe835..a5dc8d5 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -1,34 +1,31 @@ name: CI -# WORD OF CAUTION: -# TO anyone modifying this -# Things will break if you modify this -# without understanding how it works - -# A simple flow of this file: -# Apply AutoPEP8 → Apply Plugin Metadata → CRITICAL COMMIT (format + plugin meta) -# ← ← ← ← ← ↵ -# ↪ Apply Version Metadata → Commit (version meta) → Tests +# Runs only on pushes to main - i.e. after a PR has been merged, or on a +# direct maintainer push. This is fully trusted, same-repo content, so it's +# safe for it to execute the tree and push directly. This is also the +# AUTHORITATIVE integrity check: test/test_checks.py's test_versions runs +# here unmodified/strict against real, permanent git history (unlike +# ci-check.yml, which can't yet resolve a commit sha for a brand-new plugin +# version and runs leniently instead). on: push: branches: - main - pull_request_target: + +permissions: + contents: write jobs: build: runs-on: ubuntu-latest steps: - - uses: actions/checkout@v6 + - uses: actions/checkout@v7 with: - token: ${{ secrets.GITHUB_TOKEN }} - repository: ${{ github.event.pull_request.head.repo.full_name }} - ref: ${{ github.head_ref }} fetch-depth: 0 - name: Set up Python - uses: actions/setup-python@v6 + uses: actions/setup-python@v7 with: python-version: "3.12" @@ -42,21 +39,10 @@ jobs: run: | autopep8 --in-place --recursive --max-line-length=100 . - - name: Apply Plugin Metadata - if: github.event_name == 'pull_request_target' - env: - GH_TOKEN: ${{ github.token }} - run: | - CHANGED_FILES=$(gh api "repos/${{ github.repository }}/pulls/${{ github.event.pull_request.number }}/files" --jq '.[].filename') - python test/auto_apply_plugin_metadata.py "$CHANGED_FILES" - - # This is a CRITICAL COMMIT for the next step - # which bases this as the commit to get the sha to store in index.json or plugin.json - - name: Commit Plugin Metadata and AutoPEP8 + - name: Commit AutoPEP8 formatting uses: stefanzweifel/git-auto-commit-action@v7 with: - commit_message: "[ci] apply-plugin-metadata-and-formatting" - branch: ${{ github.head_ref }} + commit_message: "[ci] apply-formatting" - name: Apply Version Metadata run: | @@ -66,7 +52,6 @@ jobs: uses: stefanzweifel/git-auto-commit-action@v7 with: commit_message: "[ci] apply-version-metadata" - branch: ${{ github.head_ref }} - name: Execute Tests run: | diff --git a/.github/workflows/release.yml b/.github/workflows/release.yml index 00eba47..6d7c429 100644 --- a/.github/workflows/release.yml +++ b/.github/workflows/release.yml @@ -12,14 +12,14 @@ jobs: name: Create Release runs-on: ubuntu-latest steps: - - uses: actions/checkout@v6 + - uses: actions/checkout@v7 - name: Set up Python - uses: actions/setup-python@v6 + uses: actions/setup-python@v7 with: python-version: '3.12' - name: 'Get Previous tag' - uses: oprypin/find-latest-tag@v1.1.2 + uses: oprypin/find-latest-tag@v1.1.3 with: repository: ${{ github.repository }} releases-only: true # We know that all relevant tags have a GitHub release for them. diff --git a/CHANGELOG.md b/CHANGELOG.md index b305373..f1e16ad 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,37 @@ ## Plugin Manager (dd-mm-yyyy) +### 1.1.13 (23-08-2026) + +- Changelog window hard-wraps its bullets across several source lines + +### 1.1.12 (23-08-2026) + +- Fixed request timeouts being reported as an unhandled error instead of + a connection problem, which left the plugin manager stuck on a spinner +- Response bodies are now read on the network threadpool rather than on + the main thread, so a slow server no longer freezes the game +- Index and changelog setup no longer deadlock after a failed fetch +- Fixed the plugin manager failing to load or refresh on networks where + one of GitHub's edge servers is unreachable; all of the published + addresses are now tried instead of a single one +- The DNS block workaround now corrects name resolution rather than + replacing the HTTP and TLS stack, so ordinary requests take the + standard code path +- Fixed the changelog window erroring out after the settings window had + been opened + +### 1.1.11 (09-08-2026) + +- Switched to babase.app.asyncio_loop and babase.app.threadpool + for concurrent execution +- A new threadpool to avoid blocking the babase.app.threadpool + which is for short parallel tasks +- Added a shutdown task to shutdown our threadpool + +### 1.1.10 (12-06-2026) + +- Fix for older bs versions using `EXPORT_CLASS_NAME_SHORTCUTS` import + ### 1.1.9 (11-06-2026) - Fix for bs 1.7.63 diff --git a/index.json b/index.json index 1c88a45..d879458 100644 --- a/index.json +++ b/index.json @@ -1,6 +1,30 @@ { "plugin_manager_url": "https://github.com/bombsquad-community/plugin-manager/{content_type}/{tag}/plugin_manager.py", "versions": { + "1.1.13": { + "api_version": 9, + "commit_sha": "0bd5647", + "released_on": "23-08-2026", + "md5sum": "6f4f6e7003180491c35d6cfd5ce2f713" + }, + "1.1.12": { + "api_version": 9, + "commit_sha": "b03ab3a", + "released_on": "23-08-2026", + "md5sum": "ae9724579bcf54c29d36b6c65cbccb06" + }, + "1.1.11": { + "api_version": 9, + "commit_sha": "a74246d", + "released_on": "09-08-2026", + "md5sum": "1dbf57b11602a22196711bdd3ad18fdc" + }, + "1.1.10": { + "api_version": 9, + "commit_sha": "a1baa5f", + "released_on": "12-06-2026", + "md5sum": "ee4f7e3e88c2982bc8290e204b10b050" + }, "1.1.9": { "api_version": 9, "commit_sha": "3adfe38", diff --git a/plugin_manager.py b/plugin_manager.py index 5fb639d..53f46aa 100644 --- a/plugin_manager.py +++ b/plugin_manager.py @@ -2,23 +2,24 @@ import babase import bauiv1 as bui from bauiv1lib import popup, confirm -from babase._meta import _DEPRECATED_EXPORT_SHORTCUTS from bauiv1lib.settings.allsettings import AllSettingsWindow +import urllib.error import urllib.request import http.client import socket import json -import ssl import re import os -import sys import copy import asyncio import pathlib import hashlib +import weakref +import threading import contextlib +import concurrent.futures from typing import override from datetime import datetime @@ -26,7 +27,7 @@ from datetime import datetime # Modules used for overriding AllSettingsWindow import logging -PLUGIN_MANAGER_VERSION = "1.1.9" +PLUGIN_MANAGER_VERSION = "1.1.13" REPOSITORY_URL = "https://github.com/bombsquad-community/plugin-manager" # Current tag can be changed to "staging" or any other branch in # plugin manager repo for testing purpose. @@ -44,11 +45,39 @@ HEADERS = { } PLUGIN_DIRECTORY = _env["python_directory_user"] -# compatibility for older API versions. -if _env.get("build_number", 0) < 22714: - babase._asyncio._g_asyncio_event_loop = babase._asyncio._asyncio_event_loop +NETWORK_REQUEST_TIMEOUT = 10 # seconds + +loop = babase.app.asyncio_loop +pool = babase.app.threadpool + +# babase.app.threadpool (aka `pool`, also wired up as the asyncio loop's +# default executor) is reserved for short parallel work and warns/starves +# on long-running tasks. Network requests routinely run past that, so they +# get their own small dedicated pool instead. +_network_pool = concurrent.futures.ThreadPoolExecutor( + max_workers=4, + thread_name_prefix="PluginManagerNetwork", +) + + +async def _shutdown_network_pool() -> None: + """Drain _network_pool's workers so none outlive app shutdown. + + ThreadPoolExecutor.shutdown(wait=True) blocks, so it's run on a + throwaway thread and awaited from here rather than blocking the + event loop (other shutdown tasks run concurrently with this one). + In-flight requests are bounded by NETWORK_REQUEST_TIMEOUT, so this + settles quickly. + """ + done = asyncio.Event() + + def _join() -> None: + _network_pool.shutdown(wait=True, cancel_futures=True) + loop.call_soon_threadsafe(done.set) + + threading.Thread(target=_join, daemon=True).start() + await done.wait() -loop = babase._asyncio._g_asyncio_event_loop open_popups = [] @@ -77,12 +106,48 @@ def _by_scale(a, b, c): ) +def _wrap_markdown_bullets(source_lines, max_width, scale): + """Reflow markdown bullets so every rendered line fits `max_width`. + + CHANGELOG.md hard-wraps its bullets across several source lines, so the + lines are first stitched back into whole bullets and then re-wrapped to + the width we actually have. Drawing the source lines as-is left each one + a different length, and since a text widget shrinks itself to its own + maxwidth, long lines rendered visibly smaller than short ones. + """ + bullets = [] + for line in source_lines: + stripped = line.strip() + if not stripped: + continue + if stripped.startswith(("-", "*")) or not bullets: + bullets.append(stripped.lstrip("-*").strip()) + else: + # A continuation of the bullet above it. + bullets[-1] += " " + stripped + + lines = [] + for bullet in bullets: + prefix, current = "- ", "" + for word in bullet.split(): + candidate = f"{current} {word}" if current else word + width = bui.get_string_width(prefix + candidate, suppress_warning=True) + if current and width * scale > max_width: + lines.append(prefix + current) + # Indent wrapped remainders under the bullet's text. + prefix, current = " ", word + else: + current = candidate + lines.append(prefix + current) + return lines + + REGEXP = { "plugin_api_version": re.compile(b"(?<=ba_meta require api )(.*)"), "plugin_entry_points": re.compile( bytes( "(ba_meta export (plugin|{})\n+class )(.*)\\(".format( - _regexp_friendly_class_name_shortcut(_DEPRECATED_EXPORT_SHORTCUTS["plugin"]), + _regexp_friendly_class_name_shortcut("babase.Plugin"), ), "utf-8" ), @@ -126,26 +191,57 @@ class CategoryMetadataParseError(Exception): pass +@contextlib.contextmanager +def network_errors_as_urlerror(): + """Normalize network failures into urllib.error.URLError. + + Every caller in here reports connectivity problems by catching + URLError, but urllib only guarantees that shape while it is building + the request. Once urlopen() has returned, a socket timeout surfaces + as a bare TimeoutError and a truncated/dropped response as an + http.client.HTTPException, both of which sail past those handlers. + Since NETWORK_REQUEST_TIMEOUT made timeouts reachable at all, wrap + the whole fetch so callers only ever have one exception to catch. + """ + try: + yield + except urllib.error.URLError: + # Includes HTTPError; already the shape callers expect. + raise + except (TimeoutError, http.client.HTTPException) as e: + raise urllib.error.URLError(e) from e + + def send_network_request(request): - return urllib.request.urlopen(request) + """Fetch `request` and return its full body as bytes. + + The body is read here rather than handed back unread, because reading + it is itself a blocking call that can time out, and callers await this + from the event loop thread, where doing so would both stall the game + and raise outside network_errors_as_urlerror()'s reach. + """ + with network_errors_as_urlerror(): + with urllib.request.urlopen(request, timeout=NETWORK_REQUEST_TIMEOUT) as response: + return response.read() async def async_send_network_request(request): - response = await loop.run_in_executor(None, send_network_request, request) - return response + content = await loop.run_in_executor(_network_pool, send_network_request, request) + return content def stream_network_response_to_file(request, file, md5sum=None, retries=3): - response = urllib.request.urlopen(request) chunk_size = 16 * 1024 content = b"" - with open(file, "wb") as fout: - while True: - chunk = response.read(chunk_size) - if not chunk: - break - fout.write(chunk) - content += chunk + with network_errors_as_urlerror(): + with urllib.request.urlopen(request, timeout=NETWORK_REQUEST_TIMEOUT) as response: + with open(file, "wb") as fout: + while True: + chunk = response.read(chunk_size) + if not chunk: + break + fout.write(chunk) + content += chunk if md5sum and hashlib.md5(content).hexdigest() != md5sum: if retries <= 0: raise MD5CheckSumFailed("MD5 checksum match failed.") @@ -161,7 +257,7 @@ def stream_network_response_to_file(request, file, md5sum=None, retries=3): async def async_stream_network_response_to_file(request, file, md5sum=None, retries=3): content = await loop.run_in_executor( - None, + _network_pool, stream_network_response_to_file, request, file, @@ -192,50 +288,108 @@ class DNSBlockWorkaround: Usage: ----- >>> import urllib.request - >>> import http.client >>> import socket - >>> import ssl >>> import json >>> DNSBlockWorkaround.apply() >>> response = urllib.request.urlopen("https://dnsblockeddomain.com/path/to/resource/") """ - _google_dns_cache = {} + # Hostnames worth second-guessing the system resolver on. Keeping this + # explicit is what lets apply() patch a process-global function safely: + # every other name the game looks up takes a set membership test and is + # then handed straight to the original resolver. + _blockable_hosts = frozenset(("raw.githubusercontent.com",)) - def apply(): - opener = urllib.request.build_opener( - DNSBlockWorkaround._HTTPHandler, - DNSBlockWorkaround._HTTPSHandler, + # Maps a hostname to the addresses to dial for it, or to None when the + # host resolves normally and needs no workaround at all. Only populated + # for hosts we've already checked, so the check happens once per session. + _resolution_cache = {} + + _original_getaddrinfo = None + + @classmethod + def apply(cls): + """Correct socket.getaddrinfo() instead of rebuilding the HTTP stack. + + The block only ever corrupts one thing, the answer the resolver + hands back, so that is the only thing worth replacing. Fixing it + here leaves urllib, http.client and ssl completely stock: + socket.create_connection() still does its own address walking, + IPv6 handling and error aggregation, and TLS still verifies + against the hostname rather than whatever address we dialed. + """ + if cls._original_getaddrinfo is not None: + # Already applied. Patching again would nest the wrapper. + return + cls._original_getaddrinfo = staticmethod(socket.getaddrinfo) + socket.getaddrinfo = cls._getaddrinfo + + @classmethod + def _getaddrinfo(cls, host, port, family=0, type=0, proto=0, flags=0): + # Signature and argument names mirror socket.getaddrinfo(), which + # callers pass both positionally and by keyword. + if host in cls._blockable_hosts: + addresses = cls._addresses_to_dial(host) + if addresses is not None: + # Re-resolving each literal address is just parsing; it + # builds the 5-tuples callers expect without a lookup. + return [ + addrinfo + for address in addresses + for addrinfo in cls._original_getaddrinfo( + address, port, family, type, proto, flags) + ] + return cls._original_getaddrinfo(host, port, family, type, proto, flags) + + @classmethod + def _resolve_using_google_dns(cls, hostname): + response = urllib.request.urlopen( + f"https://dns.google/resolve?name={hostname}", + timeout=NETWORK_REQUEST_TIMEOUT, ) - urllib.request.install_opener(opener) - - def _resolve_using_google_dns(hostname): - response = urllib.request.urlopen(f"https://dns.google/resolve?name={hostname}") response = response.read() response = json.loads(response) - resolved_host = response["Answer"][0]["data"] - return resolved_host + # Answers can include CNAME records (type 5) alongside the A (1) and + # AAAA (28) records; only the latter are dialable. + return [answer["data"] for answer in response.get("Answer", ()) + if answer.get("type") in (1, 28)] - def _resolve_using_system_dns(hostname): + @classmethod + def _resolve_using_system_dns(cls, hostname): resolved_host = socket.gethostbyname(hostname) return resolved_host - def _resolve_with_workaround(hostname): - resolved_host_from_cache = DNSBlockWorkaround._google_dns_cache.get(hostname) - if resolved_host_from_cache: - return resolved_host_from_cache + @classmethod + def _addresses_to_dial(cls, hostname): + """Addresses to substitute for `hostname`, or None to leave it alone. - resolved_host_by_system_dns = DNSBlockWorkaround._resolve_using_system_dns(hostname) + Returning None is the common case and means the system resolver's + answer stands. Note that both branches yield *every* usable address + rather than one: socket.create_connection() walks the list until + something answers, and raw.githubusercontent.com publishes four A + records whose edge nodes are not all reachable from every network, + so collapsing to a single address would fail a share of requests + outright. + """ + if hostname in cls._resolution_cache: + return cls._resolution_cache[hostname] - if DNSBlockWorkaround._is_blocked(hostname, resolved_host_by_system_dns): - resolved_host = DNSBlockWorkaround._resolve_using_google_dns(hostname) - DNSBlockWorkaround._google_dns_cache[hostname] = resolved_host + try: + resolved_host_by_system_dns = cls._resolve_using_system_dns(hostname) + except socket.gaierror: + # A block that answers with NXDOMAIN rather than a bogus address. + addresses = cls._resolve_using_google_dns(hostname) or None else: - resolved_host = resolved_host_by_system_dns + if cls._is_blocked(hostname, resolved_host_by_system_dns): + addresses = cls._resolve_using_google_dns(hostname) or None + else: + addresses = None - return resolved_host + cls._resolution_cache[hostname] = addresses + return addresses - def _is_blocked(hostname, address): + @classmethod + def _is_blocked(cls, hostname, address): is_blocked = False if hostname == "raw.githubusercontent.com": # Jio's DNS server may be blocking it. @@ -243,36 +397,6 @@ class DNSBlockWorkaround: return is_blocked - class _HTTPConnection(http.client.HTTPConnection): - def connect(self): - host = DNSBlockWorkaround._resolve_with_workaround(self.host) - self.sock = socket.create_connection( - (host, self.port), - self.timeout, - ) - - class _HTTPSConnection(http.client.HTTPSConnection): - def connect(self): - host = DNSBlockWorkaround._resolve_with_workaround(self.host) - sock = socket.create_connection( - (host, self.port), - self.timeout, - ) - context = ssl.SSLContext(ssl.PROTOCOL_TLS_CLIENT) - context.verify_mode = ssl.CERT_REQUIRED - context.check_hostname = True - context.load_default_certs() - sock = context.wrap_socket(sock, server_hostname=self.host) - self.sock = sock - - class _HTTPHandler(urllib.request.HTTPHandler): - def http_open(self, req): - return self.do_open(DNSBlockWorkaround._HTTPConnection, req) - - class _HTTPSHandler(urllib.request.HTTPSHandler): - def https_open(self, req): - return self.do_open(DNSBlockWorkaround._HTTPSConnection, req) - class StartupTasks: def __init__(self): @@ -419,8 +543,8 @@ class Category: self.meta_url.format(content_type="raw", tag=self.tag), headers=self.request_headers, ) - response = await async_send_network_request(request) - self._metadata = json.loads(response.read()) + content = await async_send_network_request(request) + self._metadata = json.loads(content) self.set_category_global_cache("metadata", self._metadata) return self @@ -579,7 +703,7 @@ class PluginLocal: if not self.is_installed: raise PluginNotInstalled("Plugin is not available locally.") - self._content = await loop.run_in_executor(None, self._get_content) + self._content = await loop.run_in_executor(pool, self._get_content) return self._content async def get_api_version(self): @@ -660,7 +784,7 @@ class PluginLocal: self.save() def load_plugin(self, entry_point): - plugin_class = babase._general.getclass(entry_point, babase.Plugin) + plugin_class = babase.getclass(entry_point, babase.Plugin) loaded_plugin_instance = plugin_class() loaded_plugin_instance.on_app_running() @@ -684,7 +808,7 @@ class PluginLocal: async def set_content(self, content): if not self._content: - await loop.run_in_executor(None, self._set_content, content) + await loop.run_in_executor(pool, self._set_content, content) self._content = content return self @@ -706,14 +830,23 @@ class PluginLocal: class PluginVersion: def __init__(self, plugin, version, tag=CURRENT_TAG): self.number, info = version - self.plugin = plugin + # Plugin already owns its PluginVersions (via `versions`, + # `latest_version`, `latest_compatible_version`); holding a strong + # back-reference here would form a Plugin<->PluginVersion cycle + # that only the cyclic GC can free. A weakref avoids that so + # they're freed by refcounting alone. + self._plugin_ref = weakref.ref(plugin) self.api_version = info["api_version"] self.released_on = info["released_on"] self.commit_sha = info["commit_sha"] self.md5sum = info["md5sum"] - self.download_url = self.plugin.url.format(content_type="raw", tag=tag) - self.view_url = self.plugin.url.format(content_type="blob", tag=tag) + self.download_url = plugin.url.format(content_type="raw", tag=tag) + self.view_url = plugin.url.format(content_type="blob", tag=tag) + + @property + def plugin(self): + return self._plugin_ref() def __eq__(self, plugin_version): return (self.number, self.plugin.name) == (plugin_version.number, @@ -868,9 +1001,11 @@ class PluginManager: def __init__(self): self.request_headers = HEADERS self._index = _CACHE.get("index", {}) - self._changelog = _CACHE.get("changelog", {}) + # The raw changelog text, kept separate from the parsed entry in + # _CACHE["changelog"]; see setup_changelog(). + self._changelog = _CACHE.get("changelog_source") self.categories = {} - self.module_path = sys.modules[__name__].__file__ + self.module_path = __file__ self._index_setup_in_progress = False self._changelog_setup_in_progress = False @@ -884,8 +1019,8 @@ class PluginManager: ), headers=self.request_headers, ) - response = await async_send_network_request(request) - index = json.loads(response.read()) + content = await async_send_network_request(request) + index = json.loads(content) self.set_index_global_cache(index) self._index = index return self._index @@ -896,12 +1031,17 @@ class PluginManager: # Rather wait for the previous network call to complete. await asyncio.sleep(0.1) self._index_setup_in_progress = not bool(self._index) - index = await self.get_index() - await self.setup_plugin_categories(index) - self._index_setup_in_progress = False + try: + index = await self.get_index() + await self.setup_plugin_categories(index) + finally: + # Must clear even when the setup raised, or every later call + # spins in the loop above forever waiting on a call that has + # already given up. + self._index_setup_in_progress = False - async def get_changelog(self) -> list[str, bool]: - requested = False + async def get_changelog(self) -> str: + """The full CHANGELOG.md text, fetched once per session.""" if not self._changelog: request = urllib.request.Request(CHANGELOG_META.format( repository_url=REPOSITORY_URL, @@ -909,10 +1049,10 @@ class PluginManager: tag=CURRENT_TAG ), headers=self.request_headers) - response = await async_send_network_request(request) - self._changelog = response.read().decode() - requested = True - return [self._changelog, requested] + content = await async_send_network_request(request) + self._changelog = content.decode() + self.set_changelog_source_global_cache(self._changelog) + return self._changelog async def setup_changelog(self, version=None) -> None: if version is None: @@ -923,14 +1063,18 @@ class PluginManager: await asyncio.sleep(0.1) self._changelog_setup_in_progress = not bool(self._changelog) try: - full_changelog = await self.get_changelog() - # check if the changelog was requested - if full_changelog[1]: + try: + # Parsing is pure string work on text we already hold, so it + # runs every time rather than only on the call that fetched. + # Skipping it was what let the raw text reach the cache in + # place of the parsed entry ChangelogWindow reads. + full_changelog = await self.get_changelog() pattern = rf"### {version} \(\d\d-\d\d-\d{{4}}\)\n(.*?)(?=### \d+\.\d+\.\d+|\Z)" - if (len(full_changelog[0].split(version)) > 1): - released_on = full_changelog[0].split(version)[1].split('\n')[0] - matches = re.findall(pattern, full_changelog[0], re.DOTALL) + if (len(full_changelog.split(version)) > 1): + released_on = full_changelog.split(version)[1].split('\n')[0] + matches = re.findall(pattern, full_changelog, re.DOTALL) else: + released_on = ' (Not Provided)' matches = None if matches: @@ -939,15 +1083,17 @@ class PluginManager: 'info': matches[0].strip() } else: - changelog = {'released_on': ' (Not Provided)', + changelog = {'released_on': released_on, 'info': f"Changelog entry for version {version} not found."} - else: - changelog = full_changelog[0] - except urllib.error.URLError: - changelog = {'released_on': ' (Not Provided)', - 'info': 'Could not get ChangeLog due to Internet Issues.'} - self.set_changelog_global_cache(changelog) - self._changelog_setup_in_progress = False + except urllib.error.URLError: + changelog = {'released_on': ' (Not Provided)', + 'info': 'Could not get ChangeLog due to Internet Issues.'} + self.set_changelog_global_cache(changelog) + finally: + # Must clear even when the setup raised, or every later call + # spins in the loop above forever waiting on a call that has + # already given up. + self._changelog_setup_in_progress = False async def setup_plugin_categories(self, plugin_index): # A hack to have the "All" category show at the top. @@ -999,12 +1145,12 @@ class PluginManager: def set_changelog_global_cache(self, changelog): _CACHE["changelog"] = changelog + def set_changelog_source_global_cache(self, changelog_source): + _CACHE["changelog_source"] = changelog_source + def unset_index_global_cache(self): - try: - del _CACHE["index"] - del _CACHE["changelog"] - except KeyError: - pass + for key in ("index", "changelog", "changelog_source"): + _CACHE.pop(key, None) async def get_update_details(self): index = await self.get_index() @@ -1034,8 +1180,7 @@ class PluginManager: content_type="raw", tag=tag, ) - response = await async_send_network_request(download_url) - content = response.read() + content = await async_send_network_request(download_url) if hashlib.md5(content).hexdigest() != to_version_info["md5sum"]: raise MD5CheckSumFailed("MD5 checksum failed during plugin manager update.") with open(self.module_path, "wb") as fout: @@ -1051,7 +1196,7 @@ class ChangelogWindow(popup.PopupWindow): self.scale_origin = origin_widget.get_screen_space_center() s = 1.65 if _uiscale() is babase.UIScale.SMALL else 1.39 if _uiscale() is babase.UIScale.MEDIUM else 1.67 width = 400 * s - height = width * 0.5 + height = width * 0.6 color = (1, 1, 1) text_scale = 0.7 * s self._transition_out = 'out_scale' @@ -1097,12 +1242,10 @@ class ChangelogWindow(popup.PopupWindow): released_on = _CACHE['changelog']['released_on'] logs = _CACHE['changelog']['info'].split('\n') h_align = 'left' - extra = 0.1 except KeyError: released_on = '' logs = ["Could not load ChangeLog"] h_align = 'center' - extra = 1 bui.textwidget( parent=self._root_widget, @@ -1130,20 +1273,48 @@ class ChangelogWindow(popup.PopupWindow): ) ) - loop_height = height * 0.62 + # The entry can be arbitrarily long, so it scrolls rather than + # spilling out of the bottom of the window. + scroll_width = width * 0.88 + scroll_height = height * 0.62 + self._scrollwidget = bui.scrollwidget( + parent=self._root_widget, + size=(scroll_width, scroll_height), + position=(width * 0.06, height * 0.04), + capture_arrows=True + ) + + body_scale = text_scale * 0.7 + line_height = 36 * body_scale + text_width = scroll_width - 30 + if h_align == 'left': + # Leave headroom below `maxwidth` so no line ends up shrunk + # (and thus smaller than its neighbours) by a rounding hair. + logs = _wrap_markdown_bullets(logs, text_width * 0.95, body_scale) + + content_height = max(scroll_height, line_height * len(logs) + 20) + content = bui.containerwidget( + parent=self._scrollwidget, + size=(text_width, content_height), + background=False, + claims_left_right=False + ) + + loop_height = content_height - line_height * 0.5 - 10 for log in logs: bui.textwidget( - parent=self._root_widget, - position=(width * 0.5 * extra, loop_height), + parent=content, + position=(text_width * 0.5 if h_align == 'center' else 0, + loop_height), size=(0, 0), h_align=h_align, v_align='center', text=log, - scale=text_scale, + scale=body_scale, color=color, - maxwidth=width * 0.9 + maxwidth=text_width ) - loop_height -= 30 + loop_height -= line_height def _back(self) -> None: bui.getsound('swish').play() @@ -1736,6 +1907,7 @@ class PluginWindow(popup.PopupWindow): _remove_popup(self) bui.containerwidget(edit=self._root_widget, transition='out_scale') + @staticmethod def button(fn): async def asyncio_handler(fn, self, *args, **kwargs): await fn(self, *args, **kwargs) @@ -2261,8 +2433,8 @@ class PluginManagerWindow(bui.MainWindow): def __init__( self, - transition: str = "in_right", - origin_widget: bui.Widget = None + transition: str | None = "in_right", + origin_widget: bui.Widget | None = None ): self.plugin_manager = PluginManager() self.category_selection_button = None @@ -3420,7 +3592,7 @@ class EntryPoint(babase.Plugin): from bauiv1lib.settings import allsettings allsettings.AllSettingsWindow = NewAllSettingsWindow DNSBlockWorkaround.apply() - asyncio.set_event_loop(babase._asyncio._g_asyncio_event_loop) + babase.app.add_shutdown_task(_shutdown_network_pool()) startup_tasks = StartupTasks() loop.create_task(startup_tasks.execute()) diff --git a/test/auto_apply_plugin_metadata.py b/test/auto_apply_plugin_metadata.py index 228e810..088e3cc 100644 --- a/test/auto_apply_plugin_metadata.py +++ b/test/auto_apply_plugin_metadata.py @@ -2,11 +2,20 @@ import sys import json import ast import os +import hashlib +import subprocess import get_latest -import versioning_tools +from auto_apply_version_metadata import get_comparable_version_tuple_from_string DEBUG = True +MANIFEST_PATHS = { + "minigames": "plugins/minigames.json", + "utilities": "plugins/utilities.json", + "maps": "plugins/maps.json", + "plugman": "index.json", +} + print("DOES THIS RUN AUTO APPLY PLUGIN METADATA?") @@ -15,23 +24,69 @@ def debug_print(*args, **kwargs): print(*args, **kwargs) -def get_latest_version(plugin_name, category) -> str: - filepaths = { - "minigames": "plugins/minigames.json", - "utilities": "plugins/utilities.json", - "maps": "plugins/maps.json", - "plugman": "index.json", - } +def version_key(version): + """Comparison/sort key for a version string, so 1.0.10 sorts above 1.0.9.""" + try: + return get_comparable_version_tuple_from_string(version) + except ValueError: + raise ValueError(f"Version {version!r} is not in x.y.z form.") + +def md5sum_of(path): + with open(path, "rb") as fin: + return hashlib.md5(fin.read()).hexdigest() + + +def get_latest_version(plugin_name, category) -> str: try: if category != "plugman": - return get_latest.get_latest_plugin_version(plugin_name, filepaths[category]) + return get_latest.get_latest_plugin_version(plugin_name, MANIFEST_PATHS[category]) return get_latest.get_latest_plugman_version() except Exception as e: raise e +def read_manifest_at(path, ref): + """Load a manifest as it exists at `ref`, or None if it can't be read there.""" + try: + blob = subprocess.run( + ["git", "show", f"{ref}:{path}"], + capture_output=True, + check=True, + ).stdout + return json.loads(blob) + except (OSError, subprocess.CalledProcessError, json.JSONDecodeError): + return None + + +def get_published_versions(plugin_name, category): + """Versions of `plugin_name` that are already published on the PR's base branch. + + The working tree cannot answer this. Once ci-apply.yml has pushed its + "[ci] apply-plugin-metadata-and-formatting" commit back to the PR branch, + the PR's own copy of the manifest already lists the version being added - + so comparing against the working tree rejects every re-run of PR Check, + including the one ci-apply.yml's own push triggers, and every re-run caused + by a contributor pushing a follow-up commit. + + PLUGMAN_BASE_REF is set by ci-check.yml to the PR's base sha. Local runs + fall back to origin/main, then to the working tree when there is no history + to consult at all. + """ + path = MANIFEST_PATHS[category] + manifest = None + for ref in (os.environ.get("PLUGMAN_BASE_REF"), "origin/main", "main"): + if ref: + manifest = read_manifest_at(path, ref) + if manifest is not None: + break + if manifest is None: + with open(path, "r") as file: + manifest = json.load(file) + return manifest["plugins"].get(plugin_name, {}).get("versions", {}) + + def update_plugman_json(version): with open("index.json", "r+") as file: data = json.load(file) @@ -45,39 +100,58 @@ def update_plugman_json(version): data["versions"] = dict(sorted(data["versions"].items(), reverse=True)) -def update_plugin_json(plugin_info, category): +def update_plugin_json(plugin_info, category, plugin_path): name = plugin_info["plugin_name"] + version = plugin_info["version"] + + # Ensure the version is always greater than the already PUBLISHED version - + # what is on the base branch, not what this PR's own tree happens to say. + published = get_published_versions(name, category) + if published: + latest_published = max(published, key=version_key) + if version_key(version) <= version_key(latest_published): + raise Exception( + "Version cant be lower or equal than the previous version. " + f"{name} {latest_published} is already published; bump the version " + f"in its plugman dict (currently {version})." + ) with open(f"plugins/{category}.json", "r+") as file: data = json.load(file) - try: - # Check if plugin is already in the json - plugin = data["plugins"][name] - plugman_version = int( - versioning_tools.semantic_to_str(get_latest_version(name, category)) - ) - current_version = int(versioning_tools.semantic_to_str(plugin_info["version"])) - # Ensure the version is always greater from the already released version - if current_version > plugman_version: - plugin["versions"][plugin_info["version"]] = None - # Ensure latest version appears first - plugin["versions"] = dict(sorted(plugin["versions"].items(), reverse=True)) - plugin["description"] = plugin_info["description"] - plugin["external_url"] = plugin_info["external_url"] - plugin["authors"] = plugin_info["authors"] - # In future - # We can clear the version metadata for if current_version = plugman_version for PRs - # So that its easier to update the plugin after review without changing version - elif current_version <= plugman_version: - raise Exception("Version cant be lower or equal than the previous version.") - except KeyError: - data["plugins"][name] = { + plugin = data["plugins"].get(name) + if plugin is None: + # New plugin. Key order here is the shape every other entry has. + plugin = data["plugins"][name] = { "description": plugin_info["description"], "external_url": plugin_info["external_url"], "authors": plugin_info["authors"], - "versions": {plugin_info["version"]: None}, + "versions": {}, } + versions = plugin["versions"] + stamped = versions.get(version) + # A null placeholder is only (re)written when there is something for + # auto_apply_version_metadata.py to stamp, which keeps re-runs over an + # already-processed tree a no-op - otherwise ci-apply.yml's push would + # trigger a PR Check that undoes the stamp, forever. + # + # A stamped entry whose md5sum no longer matches the file means the + # contributor pushed further edits under the same UNPUBLISHED version + # (the published case raised above). Reset it so the stamp is + # recomputed, rather than demanding a bump for every review iteration. + if version not in versions or ( + isinstance(stamped, dict) and stamped.get("md5sum") != md5sum_of(plugin_path) + ): + versions[version] = None + + # Ensure latest version appears first + plugin["versions"] = dict( + sorted(versions.items(), key=lambda item: version_key(item[0]), reverse=True) + ) + plugin["description"] = plugin_info["description"] + plugin["external_url"] = plugin_info["external_url"] + plugin["authors"] = plugin_info["authors"] + file.seek(0) json.dump(data, file, indent=2, ensure_ascii=False) # Ensure old content is removed @@ -132,7 +206,7 @@ def extract_plugman(plugins): ) result[kw.arg] = ast.literal_eval(kw.value) if category: - update_plugin_json(result, category=category) + update_plugin_json(result, category=category, plugin_path=plugin) else: update_plugman_json(result) # raise ValueError("Variable plugman not found in the file or has unsupported format.") diff --git a/test/test_checks.py b/test/test_checks.py index c5bc6df..5de8010 100644 --- a/test/test_checks.py +++ b/test/test_checks.py @@ -11,6 +11,39 @@ from packaging.version import Version import unittest +# Raised by Repo.commit() when a revision cannot be resolved. Both entries are +# required because which one you get depends on how the sha is spelled: an +# unresolvable abbreviated name - the 7-8 char form this repo actually stores, +# see auto_apply_version_metadata.py - raises BadName, whereas a full-length +# but absent 40-hex sha raises plain ValueError. BadName/BadObject are NOT +# ValueError subclasses, so neither entry is redundant. Kept narrow so lenient +# mode can't mask unrelated repository errors. +# +# BadName/BadObject originate in gitdb but are re-exported by GitPython in +# git.exc.__all__, so git.exc.* is the supported public spelling and is used +# here deliberately - it avoids importing gitdb, a transitive dependency that +# test/pip_reqs.txt does not declare directly. +UNRESOLVED_COMMIT_ERRORS = (ValueError, git.exc.BadName, git.exc.BadObject) + + +def is_unpublished_version(repository, version_metadata): + """True if this version entry is being introduced by the PR under test. + + Such an entry can't satisfy the history checks yet: its metadata is either + still an unstamped ``null`` placeholder, or was stamped against the current + HEAD by ci-check.yml's preview - describing the reformatted working tree + rather than anything committed. Published entries always point at an + earlier commit whose tree really does contain the described file, so they + are unaffected. Only consulted in lenient mode (ci-check.yml); the + authoritative strict run on push-to-main still validates these. + """ + if not version_metadata or not version_metadata.get("commit_sha"): + return True + try: + return repository.commit(version_metadata["commit_sha"]) == repository.head.commit + except UNRESOLVED_COMMIT_ERRORS: + return False # unresolvable: let the caller's handler report it + class TestPluginManagerMetadata(unittest.TestCase): def setUp(self): @@ -40,8 +73,19 @@ class TestPluginManagerMetadata(unittest.TestCase): assert sorted_versions == versions def test_versions(self): + lenient = os.environ.get("PLUGMAN_CI_LENIENT_HISTORY") == "1" for version_name, version_metadata in self.content["versions"].items(): - commit = self.repository.commit(version_metadata["commit_sha"]) + if lenient and is_unpublished_version(self.repository, version_metadata): + print(f"[lenient] skipping {version_name}: not committed yet") + continue + try: + commit = self.repository.commit(version_metadata["commit_sha"]) + except UNRESOLVED_COMMIT_ERRORS as err: + if lenient: + print(f"[lenient] skipping {version_name}: commit " + f"{version_metadata['commit_sha']} not found yet ({err})") + continue + raise plugin_manager = commit.tree / self.plugin_manager with io.BytesIO(plugin_manager.data_stream.read()) as fin: content = fin.read() @@ -144,9 +188,21 @@ class BaseCategoryMetadataTestCases: self.assertTrue(len(plugin_metadata["versions"]) > 0) def test_versions(self): + lenient = os.environ.get("PLUGMAN_CI_LENIENT_HISTORY") == "1" for plugin_name, plugin_metadata in self.content["plugins"].items(): for version_name, version_metadata in plugin_metadata["versions"].items(): - commit = self.repository.commit(version_metadata["commit_sha"]) + if lenient and is_unpublished_version(self.repository, version_metadata): + print(f"[lenient] skipping {plugin_name} {version_name}: " + "not committed yet") + continue + try: + commit = self.repository.commit(version_metadata["commit_sha"]) + except UNRESOLVED_COMMIT_ERRORS as err: + if lenient: + print(f"[lenient] skipping {plugin_name} {version_name}: " + f"commit {version_metadata['commit_sha']} not found yet ({err})") + continue + raise plugin = os.path.join(self.category, f"{plugin_name}.py") plugin_commit_sha = commit.tree / plugin with io.BytesIO(plugin_commit_sha.data_stream.read()) as fin: