From 6f51a40a7cf971fa26505f33ceba3d0ed5c55d6a Mon Sep 17 00:00:00 2001 From: TMHSDigital <154358121+TMHSDigital@users.noreply.github.com> Date: Sun, 4 Oct 2026 19:39:31 -0400 Subject: [PATCH] fix(scripts): refuse destructive --out targets and skip fork PR labelling build_plugin_dist.py --out rmtree'd any existing directory with no prompt; `--out .` would have wiped the checkout. It now refuses the repo, any parent of it, any git checkout, and a non-empty directory unless --force is given. Release builds into a fresh $RUNNER_TEMP path and is unaffected. tests/test_build_plugin_dist.py covers each refusal and runs in Validate; the repo-root cases test unsafe_out() directly so a guard regression cannot delete the checkout under test. label-sync's gh pr edit 403s on every fork PR (a fork's token is read-only), and release-gate.sh refuses to release a merge whose PR has a failed check, so the first outside contribution would have blocked releases. The job now skips forks, drops its unused checkout, and no longer passes --force to gh label create, which reset existing labels' colour and description on every run. Closes #352 Closes #351 Signed-off-by: TMHSDigital <154358121+TMHSDigital@users.noreply.github.com> Co-Authored-By: Claude Opus 5.5 --- .github/workflows/label-sync.yml | 12 ++++-- .github/workflows/validate.yml | 3 ++ scripts/build_plugin_dist.py | 29 ++++++++++++- tests/test_build_plugin_dist.py | 74 ++++++++++++++++++++++++++++++++ 4 files changed, 114 insertions(+), 4 deletions(-) create mode 100644 tests/test_build_plugin_dist.py diff --git a/.github/workflows/label-sync.yml b/.github/workflows/label-sync.yml index 4cf1aec8..d73b46d3 100644 --- a/.github/workflows/label-sync.yml +++ b/.github/workflows/label-sync.yml @@ -11,10 +11,15 @@ permissions: jobs: label: name: Auto-label by path + # A fork PR's GITHUB_TOKEN is read-only whatever `permissions:` says, so + # `gh pr edit` would 403 and fail this check, and release-gate.sh refuses + # to release a merge whose PR has a failed check. Skip forks; labels are + # cosmetic. No checkout: every step talks to the API only. + if: github.event.pull_request.head.repo.full_name == github.repository runs-on: ubuntu-latest + env: + GH_REPO: ${{ github.repository }} steps: - - uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7 - - name: Get changed files id: changed env: @@ -69,7 +74,8 @@ jobs: if [ -n "$LABELS" ]; then for label in $LABELS; do - gh label create "$label" --force --color "ededed" 2>/dev/null || true + # No --force: it would reset an existing label's colour and description. + gh label create "$label" --color "ededed" 2>/dev/null || true gh pr edit ${{ github.event.pull_request.number }} --add-label "$label" done echo "Applied labels:$LABELS" diff --git a/.github/workflows/validate.yml b/.github/workflows/validate.yml index 7111688e..d3a8b991 100644 --- a/.github/workflows/validate.yml +++ b/.github/workflows/validate.yml @@ -395,6 +395,9 @@ jobs: - name: Release notes generator run: python3 tests/test_release_notes.py -v + - name: Plugin-dist --out refuses destructive targets + run: python3 tests/test_build_plugin_dist.py -v + - name: Hero-drift tool exit-code tests run: | pip install numpy==2.2.6 Pillow==11.2.1 diff --git a/scripts/build_plugin_dist.py b/scripts/build_plugin_dist.py index 7b9b5402..419061f1 100644 --- a/scripts/build_plugin_dist.py +++ b/scripts/build_plugin_dist.py @@ -8,7 +8,8 @@ skills/ claude/ rules/ snippets/ templates/ LICENSE files .claude-plugin/plugin.json + marketplace.json, and a short README - python scripts/build_plugin_dist.py --out DIR # build + python scripts/build_plugin_dist.py --out DIR # build (DIR must be absent or empty) + python scripts/build_plugin_dist.py --out DIR --force # replace a previous build in DIR python scripts/build_plugin_dist.py --check # build to a temp dir, verify, report size release.yml force-pushes the built tree as a single orphan commit to @@ -45,6 +46,25 @@ """ +def unsafe_out(out: Path, force: bool) -> str | None: + """Why `out` must not be deleted, or None when building there is safe. + + build() replaces `out` wholesale, so refuse anything that is the repo, + contains the repo, or is a git checkout, and only replace a non-empty + directory when the caller asked for it with --force. + """ + out = out.resolve() + if out == ROOT or out in ROOT.parents: + return f"{out} is the repository or one of its parents" + if (out / ".git").exists(): + return f"{out} contains a .git directory" + if out.exists() and not out.is_dir(): + return f"{out} exists and is not a directory" + if out.is_dir() and any(out.iterdir()) and not force: + return f"{out} is not empty; pass --force to replace it" + return None + + def build(out: Path) -> int: if out.exists(): shutil.rmtree(out) @@ -81,9 +101,16 @@ def main(argv=None) -> int: ap = argparse.ArgumentParser(description=__doc__.splitlines()[0]) ap.add_argument("--out", type=Path) ap.add_argument("--check", action="store_true") + ap.add_argument("--force", action="store_true", + help="replace a non-empty --out directory") a = ap.parse_args(argv) if not a.out and not a.check: ap.error("give --out DIR or --check") + if a.out: + reason = unsafe_out(a.out, a.force) + if reason: + print(f"::error::refusing to build into {reason}", file=sys.stderr) + return 2 with tempfile.TemporaryDirectory() as td: out = a.out or Path(td) / "dist" size = build(out) diff --git a/tests/test_build_plugin_dist.py b/tests/test_build_plugin_dist.py new file mode 100644 index 00000000..e31d5e7a --- /dev/null +++ b/tests/test_build_plugin_dist.py @@ -0,0 +1,74 @@ +"""Safety tests for scripts/build_plugin_dist.py --out. No Blender required. + +build() replaces --out wholesale, so the script must refuse a target it +would destroy (the repo, a parent of it, a git checkout, or a non-empty +directory without --force) before touching anything. Run: + python tests/test_build_plugin_dist.py -v +""" +from __future__ import annotations + +import contextlib +import io +import sys +import tempfile +import unittest +from pathlib import Path + +REPO = Path(__file__).resolve().parent.parent +sys.path.insert(0, str(REPO / "scripts")) + +import build_plugin_dist as b # noqa: E402 + + +def run(argv): + err = io.StringIO() + with contextlib.redirect_stderr(err), contextlib.redirect_stdout(io.StringIO()): + code = b.main(argv) + return code, err.getvalue() + + +class OutGuard(unittest.TestCase): + # The repo and its parents are checked through unsafe_out() directly, never + # through main(): if the guard regressed, main() would delete the checkout. + def test_refuses_repo_root(self): + self.assertIn("repository", b.unsafe_out(REPO, force=True) or "") + + def test_refuses_parent_of_repo(self): + self.assertIn("repository", b.unsafe_out(REPO.parent, force=True) or "") + + def test_refuses_non_empty_dir_without_force(self): + with tempfile.TemporaryDirectory() as td: + keep = Path(td) / "important.txt" + keep.write_text("keep", encoding="utf-8") + code, err = run(["--out", td]) + self.assertEqual(code, 2) + self.assertIn("--force", err) + self.assertTrue(keep.is_file()) + + def test_refuses_git_checkout_even_with_force(self): + with tempfile.TemporaryDirectory() as td: + (Path(td) / ".git").mkdir() + code, err = run(["--out", td, "--force"]) + self.assertEqual(code, 2) + self.assertIn(".git", err) + self.assertTrue((Path(td) / ".git").is_dir()) + + def test_builds_into_missing_dir(self): + with tempfile.TemporaryDirectory() as td: + out = Path(td) / "dist" + code, _ = run(["--out", str(out)]) + self.assertEqual(code, 0) + self.assertTrue((out / ".claude-plugin" / "plugin.json").is_file()) + + def test_force_replaces_previous_build(self): + with tempfile.TemporaryDirectory() as td: + out = Path(td) / "dist" + self.assertEqual(run(["--out", str(out)])[0], 0) + (out / "stale.txt").write_text("old", encoding="utf-8") + self.assertEqual(run(["--out", str(out)])[0], 2) + self.assertEqual(run(["--out", str(out), "--force"])[0], 0) + self.assertFalse((out / "stale.txt").exists()) + + +if __name__ == "__main__": + unittest.main()