Skip to content

Commit e5f7938

Browse files
TMHSDigitalclaude
andauthored
fix(scripts): refuse destructive --out targets and skip fork PR labelling (#409)
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 <noreply@anthropic.com>
1 parent 960b341 commit e5f7938

4 files changed

Lines changed: 114 additions & 4 deletions

File tree

‎.github/workflows/label-sync.yml‎

Lines changed: 9 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -11,10 +11,15 @@ permissions:
1111
jobs:
1212
label:
1313
name: Auto-label by path
14+
# A fork PR's GITHUB_TOKEN is read-only whatever `permissions:` says, so
15+
# `gh pr edit` would 403 and fail this check, and release-gate.sh refuses
16+
# to release a merge whose PR has a failed check. Skip forks; labels are
17+
# cosmetic. No checkout: every step talks to the API only.
18+
if: github.event.pull_request.head.repo.full_name == github.repository
1419
runs-on: ubuntu-latest
20+
env:
21+
GH_REPO: ${{ github.repository }}
1522
steps:
16-
- uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7
17-
1823
- name: Get changed files
1924
id: changed
2025
env:
@@ -69,7 +74,8 @@ jobs:
6974
7075
if [ -n "$LABELS" ]; then
7176
for label in $LABELS; do
72-
gh label create "$label" --force --color "ededed" 2>/dev/null || true
77+
# No --force: it would reset an existing label's colour and description.
78+
gh label create "$label" --color "ededed" 2>/dev/null || true
7379
gh pr edit ${{ github.event.pull_request.number }} --add-label "$label"
7480
done
7581
echo "Applied labels:$LABELS"

‎.github/workflows/validate.yml‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -395,6 +395,9 @@ jobs:
395395
- name: Release notes generator
396396
run: python3 tests/test_release_notes.py -v
397397

398+
- name: Plugin-dist --out refuses destructive targets
399+
run: python3 tests/test_build_plugin_dist.py -v
400+
398401
- name: Hero-drift tool exit-code tests
399402
run: |
400403
pip install numpy==2.2.6 Pillow==11.2.1

‎scripts/build_plugin_dist.py‎

Lines changed: 28 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,8 @@
88
skills/ claude/ rules/ snippets/ templates/ LICENSE files
99
.claude-plugin/plugin.json + marketplace.json, and a short README
1010
11-
python scripts/build_plugin_dist.py --out DIR # build
11+
python scripts/build_plugin_dist.py --out DIR # build (DIR must be absent or empty)
12+
python scripts/build_plugin_dist.py --out DIR --force # replace a previous build in DIR
1213
python scripts/build_plugin_dist.py --check # build to a temp dir, verify, report size
1314
1415
release.yml force-pushes the built tree as a single orphan commit to
@@ -45,6 +46,25 @@
4546
"""
4647

4748

49+
def unsafe_out(out: Path, force: bool) -> str | None:
50+
"""Why `out` must not be deleted, or None when building there is safe.
51+
52+
build() replaces `out` wholesale, so refuse anything that is the repo,
53+
contains the repo, or is a git checkout, and only replace a non-empty
54+
directory when the caller asked for it with --force.
55+
"""
56+
out = out.resolve()
57+
if out == ROOT or out in ROOT.parents:
58+
return f"{out} is the repository or one of its parents"
59+
if (out / ".git").exists():
60+
return f"{out} contains a .git directory"
61+
if out.exists() and not out.is_dir():
62+
return f"{out} exists and is not a directory"
63+
if out.is_dir() and any(out.iterdir()) and not force:
64+
return f"{out} is not empty; pass --force to replace it"
65+
return None
66+
67+
4868
def build(out: Path) -> int:
4969
if out.exists():
5070
shutil.rmtree(out)
@@ -81,9 +101,16 @@ def main(argv=None) -> int:
81101
ap = argparse.ArgumentParser(description=__doc__.splitlines()[0])
82102
ap.add_argument("--out", type=Path)
83103
ap.add_argument("--check", action="store_true")
104+
ap.add_argument("--force", action="store_true",
105+
help="replace a non-empty --out directory")
84106
a = ap.parse_args(argv)
85107
if not a.out and not a.check:
86108
ap.error("give --out DIR or --check")
109+
if a.out:
110+
reason = unsafe_out(a.out, a.force)
111+
if reason:
112+
print(f"::error::refusing to build into {reason}", file=sys.stderr)
113+
return 2
87114
with tempfile.TemporaryDirectory() as td:
88115
out = a.out or Path(td) / "dist"
89116
size = build(out)

‎tests/test_build_plugin_dist.py‎

Lines changed: 74 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,74 @@
1+
"""Safety tests for scripts/build_plugin_dist.py --out. No Blender required.
2+
3+
build() replaces --out wholesale, so the script must refuse a target it
4+
would destroy (the repo, a parent of it, a git checkout, or a non-empty
5+
directory without --force) before touching anything. Run:
6+
python tests/test_build_plugin_dist.py -v
7+
"""
8+
from __future__ import annotations
9+
10+
import contextlib
11+
import io
12+
import sys
13+
import tempfile
14+
import unittest
15+
from pathlib import Path
16+
17+
REPO = Path(__file__).resolve().parent.parent
18+
sys.path.insert(0, str(REPO / "scripts"))
19+
20+
import build_plugin_dist as b # noqa: E402
21+
22+
23+
def run(argv):
24+
err = io.StringIO()
25+
with contextlib.redirect_stderr(err), contextlib.redirect_stdout(io.StringIO()):
26+
code = b.main(argv)
27+
return code, err.getvalue()
28+
29+
30+
class OutGuard(unittest.TestCase):
31+
# The repo and its parents are checked through unsafe_out() directly, never
32+
# through main(): if the guard regressed, main() would delete the checkout.
33+
def test_refuses_repo_root(self):
34+
self.assertIn("repository", b.unsafe_out(REPO, force=True) or "")
35+
36+
def test_refuses_parent_of_repo(self):
37+
self.assertIn("repository", b.unsafe_out(REPO.parent, force=True) or "")
38+
39+
def test_refuses_non_empty_dir_without_force(self):
40+
with tempfile.TemporaryDirectory() as td:
41+
keep = Path(td) / "important.txt"
42+
keep.write_text("keep", encoding="utf-8")
43+
code, err = run(["--out", td])
44+
self.assertEqual(code, 2)
45+
self.assertIn("--force", err)
46+
self.assertTrue(keep.is_file())
47+
48+
def test_refuses_git_checkout_even_with_force(self):
49+
with tempfile.TemporaryDirectory() as td:
50+
(Path(td) / ".git").mkdir()
51+
code, err = run(["--out", td, "--force"])
52+
self.assertEqual(code, 2)
53+
self.assertIn(".git", err)
54+
self.assertTrue((Path(td) / ".git").is_dir())
55+
56+
def test_builds_into_missing_dir(self):
57+
with tempfile.TemporaryDirectory() as td:
58+
out = Path(td) / "dist"
59+
code, _ = run(["--out", str(out)])
60+
self.assertEqual(code, 0)
61+
self.assertTrue((out / ".claude-plugin" / "plugin.json").is_file())
62+
63+
def test_force_replaces_previous_build(self):
64+
with tempfile.TemporaryDirectory() as td:
65+
out = Path(td) / "dist"
66+
self.assertEqual(run(["--out", str(out)])[0], 0)
67+
(out / "stale.txt").write_text("old", encoding="utf-8")
68+
self.assertEqual(run(["--out", str(out)])[0], 2)
69+
self.assertEqual(run(["--out", str(out), "--force"])[0], 0)
70+
self.assertFalse((out / "stale.txt").exists())
71+
72+
73+
if __name__ == "__main__":
74+
unittest.main()

0 commit comments

Comments
 (0)