diff --git a/.github/PULL_REQUEST_TEMPLATE/pipeline.md b/.github/PULL_REQUEST_TEMPLATE/pipeline.md index 138ed609ac..0b08822498 100644 --- a/.github/PULL_REQUEST_TEMPLATE/pipeline.md +++ b/.github/PULL_REQUEST_TEMPLATE/pipeline.md @@ -31,7 +31,7 @@ List the impacted workflows and related automation. ## Roles (optional `🧩 Subset` CI scope) -Optional. Ignored unless a maintainer applies the **🧩 Subset** label; without the label CI uses the diff-derived role set as usual. When the label is set, CI deploys **only** the roles listed here — each must be an existing `roles/` directory, or the run fails. This gives fork PRs the manual-dispatch whitelist. See [pipeline.md](../../docs/contributing/artefact/git/pipeline.md#subset-label-). +Optional. Ignored unless a maintainer applies the **🧩 Subset** label; without the label CI uses the diff-derived role set as usual. When the label is set, CI deploys **only** the roles listed here — each must be an existing `roles/` directory, or the run fails. See [pipeline.md](../../docs/contributing/artefact/git/pipeline.md#subset-label-). ```yaml roles: diff --git a/.github/PULL_REQUEST_TEMPLATE/server.md b/.github/PULL_REQUEST_TEMPLATE/server.md index 194c2d4e1a..6828cb6f45 100644 --- a/.github/PULL_REQUEST_TEMPLATE/server.md +++ b/.github/PULL_REQUEST_TEMPLATE/server.md @@ -41,7 +41,7 @@ Integrate the change into the following services when possible: ## Roles (optional `🧩 Subset` CI scope) -Optional. Ignored unless a maintainer applies the **🧩 Subset** label; without the label CI uses the diff-derived role set as usual. When the label is set, CI deploys **only** the roles listed here — each must be an existing `roles/` directory, or the run fails. This gives fork PRs the manual-dispatch whitelist. See [pipeline.md](../../docs/contributing/artefact/git/pipeline.md#subset-label-). +Optional. Ignored unless a maintainer applies the **🧩 Subset** label; without the label CI uses the diff-derived role set as usual. When the label is set, CI deploys **only** the roles listed here — each must be an existing `roles/` directory, or the run fails. See [pipeline.md](../../docs/contributing/artefact/git/pipeline.md#subset-label-). ```yaml roles: diff --git a/.github/PULL_REQUEST_TEMPLATE/system.md b/.github/PULL_REQUEST_TEMPLATE/system.md index 88e90c3cc6..d4b4881f9a 100644 --- a/.github/PULL_REQUEST_TEMPLATE/system.md +++ b/.github/PULL_REQUEST_TEMPLATE/system.md @@ -32,7 +32,7 @@ List the impacted roles and shared components. ## Roles (optional `🧩 Subset` CI scope) -Optional. Ignored unless a maintainer applies the **🧩 Subset** label; without the label CI uses the diff-derived role set as usual. When the label is set, CI deploys **only** the roles listed here — each must be an existing `roles/` directory, or the run fails. This gives fork PRs the manual-dispatch whitelist. See [pipeline.md](../../docs/contributing/artefact/git/pipeline.md#subset-label-). +Optional. Ignored unless a maintainer applies the **🧩 Subset** label; without the label CI uses the diff-derived role set as usual. When the label is set, CI deploys **only** the roles listed here — each must be an existing `roles/` directory, or the run fails. See [pipeline.md](../../docs/contributing/artefact/git/pipeline.md#subset-label-). ```yaml roles: diff --git a/.github/PULL_REQUEST_TEMPLATE/workstation.md b/.github/PULL_REQUEST_TEMPLATE/workstation.md index 50013057c0..fc0d189fac 100644 --- a/.github/PULL_REQUEST_TEMPLATE/workstation.md +++ b/.github/PULL_REQUEST_TEMPLATE/workstation.md @@ -32,7 +32,7 @@ List the impacted roles and workstation context. ## Roles (optional `🧩 Subset` CI scope) -Optional. Ignored unless a maintainer applies the **🧩 Subset** label; without the label CI uses the diff-derived role set as usual. When the label is set, CI deploys **only** the roles listed here — each must be an existing `roles/` directory, or the run fails. This gives fork PRs the manual-dispatch whitelist. See [pipeline.md](../../docs/contributing/artefact/git/pipeline.md#subset-label-). +Optional. Ignored unless a maintainer applies the **🧩 Subset** label; without the label CI uses the diff-derived role set as usual. When the label is set, CI deploys **only** the roles listed here — each must be an existing `roles/` directory, or the run fails. See [pipeline.md](../../docs/contributing/artefact/git/pipeline.md#subset-label-). ```yaml roles: diff --git a/scripts/meta/resolve/pr/subset_roles.py b/cli/meta/ci/subset_roles.py old mode 100755 new mode 100644 similarity index 88% rename from scripts/meta/resolve/pr/subset_roles.py rename to cli/meta/ci/subset_roles.py index 92d97d0bcd..86a494cd9c --- a/scripts/meta/resolve/pr/subset_roles.py +++ b/cli/meta/ci/subset_roles.py @@ -1,8 +1,7 @@ -#!/usr/bin/env python3 """Resolve the explicit role subset declared in a pull-request body. Activated by the ``🧩 Subset`` label: the entry-pull-request workflow only -runs this script when the label is present. It gives fork contributors the +runs this module when the label is present. It gives fork contributors the selective-CI capability that ``entry-manual.yml`` (``workflow_dispatch``) grants maintainers, which forks cannot trigger. @@ -15,7 +14,12 @@ diff-derived resolver produces. Unlike the fail-safe diff resolver (which widens to ``__ALL__`` on any problem), the subset path is strict: invalid YAML, a missing/empty ``roles:`` list, or an unknown role id each abort with a clear message and a non-zero -exit, so a mistyped subset never silently deploys the wrong set. +exit, so a mistyped subset never silently deploys the wrong set. Role ids must +match ``^[a-z0-9-]+$``, which also blocks path traversal before any filesystem +lookup. + +Invoked from scripts/github/resolve/pr_affected_roles.sh via +``python -m cli.meta.ci.subset_roles``. """ import os @@ -26,12 +30,11 @@ from typing import NoReturn import yaml -REPO_ROOT = Path(__file__).resolve().parents[4] -ROLES_DIR = REPO_ROOT / "roles" +from utils import PROJECT_ROOT +from utils.cache.yaml import load_yaml_str + +ROLES_DIR = PROJECT_ROOT / "roles" FENCE_RE = re.compile(r"```[^\n]*\n(.*?)```", re.DOTALL) -# Role dir names are lowercase alphanumerics + dashes. Enforcing this on the -# untrusted PR body also blocks path traversal (no '/', no '.') before any -# filesystem lookup. NAME_RE = re.compile(r"^[a-z0-9-]+$") @@ -49,7 +52,7 @@ def extract_roles(body: str): ) for block in blocks: try: - data = yaml.safe_load(block) + data = load_yaml_str(block) except yaml.YAMLError as exc: fail(f"invalid YAML in the PR body roles block: {exc}") if isinstance(data, dict) and "roles" in data: diff --git a/docs/contributing/artefact/git/pipeline.md b/docs/contributing/artefact/git/pipeline.md index 886bacc1f9..a986f7f1a1 100644 --- a/docs/contributing/artefact/git/pipeline.md +++ b/docs/contributing/artefact/git/pipeline.md @@ -88,11 +88,7 @@ The reverse closure is implemented in [affected resolver](../../../../cli/meta/r #### Subset label 🧩 -The manual whitelist (`entry-manual.yml`, `workflow_dispatch`) is only reachable by maintainers on the upstream repository; **fork contributors cannot dispatch it**. The `🧩 Subset` label closes that gap: it lets a PR pin CI to an explicit role list declared in the PR body, so a fork PR can run the same narrowed matrix a maintainer would have dispatched manually. - -The mechanism is **opt-in and fully backwards-compatible**. Without the `🧩 Subset` label nothing changes: the diff-driven precedence above applies exactly as before. The label is what activates the subset; the role list alone does nothing. - -The `detect-affected-roles` job in [entry-pull-request-change.yml](../../../../.github/workflows/entry-pull-request-change.yml) always calls the single dispatcher [pr_affected_roles.sh](../../../../scripts/github/resolve/pr_affected_roles.sh), which branches on the label: with `🧩 Subset` it runs [subset_roles.py](../../../../scripts/meta/resolve/pr/subset_roles.py) **instead of** the diff resolver, otherwise it falls through to the unchanged diff path. The subset script reads the PR body and expects a fenced YAML block with a `roles:` list, conventionally under a `## Roles` heading: +Add a `## Roles` section with a fenced `roles:` YAML list to the PR body and apply the `🧩 Subset` label: ````markdown ## Roles @@ -105,16 +101,16 @@ roles: ``` ```` -The resolved list becomes the `whitelist` output (with `roles_only=true`), which flows through the orchestrator into the deploy tests as precedence rung 2 above (explicit whitelist wins over the diff). The first fenced block whose YAML carries a `roles:` key is used, so the heading text is not significant. +With the label set, the `detect-affected-roles` job in [entry-pull-request-change.yml](../../../../.github/workflows/entry-pull-request-change.yml) calls [pr_affected_roles.sh](../../../../scripts/github/resolve/pr_affected_roles.sh), which runs the [cli.meta.ci.subset_roles](../../../../cli/meta/ci/subset_roles.py) module. The module reads the first fenced block whose YAML carries a `roles:` key (the heading text is not matched) and emits that list as the `whitelist` output with `roles_only=true`, which flows into the deploy tests as precedence rung 2 above. Without the label, the diff-driven precedence applies. -Unlike the diff resolver (which fail-safe widens to `__ALL__` on any problem), the subset path is **strict** and fails the run with a clear message when: +The subset path fails the run when: - the YAML is invalid, - no `roles:` block is found, - the `roles:` list is empty, or - any listed id is not an existing `roles/` directory. -This prevents a mistyped subset from silently deploying the wrong set. The `🧩 Subset` label, like `🛡️ Trusted`, must exist in the repository's label set and is applied through the PR labels UI; both the `pull_request` and `pull_request_target` events carry a `labeled` trigger, so adding it starts a fresh run that observes the subset. Because applying labels requires triage/write access, in practice a maintainer applies it after the contributor has filled in the `## Roles` block. +The `🧩 Subset` label must exist in the repository's label set and is applied through the PR labels UI. Both the `pull_request` and `pull_request_target` events carry a `labeled` trigger, so adding it starts a fresh run. ### 10. Installation Tests 📦 diff --git a/docs/contributing/tools/github/actions/workflows.md b/docs/contributing/tools/github/actions/workflows.md index 2818bbf234..2b20ea6545 100644 --- a/docs/contributing/tools/github/actions/workflows.md +++ b/docs/contributing/tools/github/actions/workflows.md @@ -25,7 +25,7 @@ Trigger column legend: **auto** = fires automatically (push, pull_request, sched | Workflow | Description | Trigger | Inputs | |---|---|---|---| -| [entry-pull-request-change.yml](../../../../../.github/workflows/entry-pull-request-change.yml): `⚡ CI: Pull Request` | Detects PR scope from changed files + branch prefix, then conditionally calls the orchestrator. A maintainer `🛡️ Trusted` label builds the privileged images from the PR head — see [pipeline.md](../../../artefact/git/pipeline.md#trusted-fork-prs-). A `🧩 Subset` label pins the deploy matrix to the roles listed in the PR body (giving fork PRs the manual-dispatch whitelist) — see [pipeline.md](../../../artefact/git/pipeline.md#subset-label-). | auto (`pull_request` & `pull_request_target`: opened, synchronize, reopened, ready_for_review, labeled) | none | +| [entry-pull-request-change.yml](../../../../../.github/workflows/entry-pull-request-change.yml): `⚡ CI: Pull Request` | Detects PR scope from changed files + branch prefix, then conditionally calls the orchestrator. A maintainer `🛡️ Trusted` label builds the privileged images from the PR head — see [pipeline.md](../../../artefact/git/pipeline.md#trusted-fork-prs-). A `🧩 Subset` label pins the deploy matrix to the roles listed in the PR body — see [pipeline.md](../../../artefact/git/pipeline.md#subset-label-). | auto (`pull_request` & `pull_request_target`: opened, synchronize, reopened, ready_for_review, labeled) | none | | [entry-push-latest.yml](../../../../../.github/workflows/entry-push-latest.yml): `⚡ CI: Push` | Syncs `main` from the configured source repository, then runs the orchestrator on pushes to `main`, `feature/**`, `hotfix/**`, `fix/**`, `chore/**`, `alert-autofix-*`; on tagged `main` it also calls `release-version.yml`. Respects the `CI_CANCEL_IN_PROGRESS`, `CI_SYNC_MAIN_SOURCE_REPOSITORY`, and `CI_RUN_ON_MAIN` repository variables, see [configuration.md](configuration.md). | auto (`push`) | none | | [entry-manual.yml](../../../../../.github/workflows/entry-manual.yml): `⚡ CI: Manual` | Manual dispatch of the full orchestrator for a chosen distro set and app whitelist. The `whitelist` input accepts the `__ALL__` sentinel for forced full deploy; see the "Diff-driven app selection" subsection in [pipeline.md](../../../artefact/git/pipeline.md). | manual | `distros` (default `debian`), `whitelist` (optional; `__ALL__` forces full deploy, empty triggers diff derivation, any other value is verbatim) | | [entry-pull-request-cancel.yml](../../../../../.github/workflows/entry-pull-request-cancel.yml): `🚫 Cancel: PR Runs on Close` | Cancels active workflow runs for a PR when it is closed or converted to draft. | auto (`pull_request_target`: closed, converted_to_draft) | none | diff --git a/scripts/github/resolve/pr_affected_roles.sh b/scripts/github/resolve/pr_affected_roles.sh index 0ae2c3be2c..bdbab69c0d 100755 --- a/scripts/github/resolve/pr_affected_roles.sh +++ b/scripts/github/resolve/pr_affected_roles.sh @@ -8,9 +8,9 @@ # Inputs via env: # SUBSET_LABELED "true" when the PR carries the '🧩 Subset' label. # When set, the explicit role list declared in the PR -# body wins, resolved by -# scripts/meta/resolve/pr/subset_roles.py (strict: it -# fails the run on invalid/empty/unknown roles). +# body wins, resolved by the cli.meta.ci.subset_roles +# module (strict: it fails the run on +# invalid/empty/unknown roles). # Otherwise the diff-derived resolver runs unchanged. # PR_BODY PR body markdown (only read on the subset path). # @@ -24,7 +24,7 @@ set -euo pipefail if [[ "${SUBSET_LABELED:-}" == "true" ]]; then echo "🧩 Subset label present: restricting CI to the roles listed in the PR body." - exec python scripts/meta/resolve/pr/subset_roles.py + exec python -m cli.meta.ci.subset_roles fi resolved="$(./scripts/meta/resolve/diff/affected_roles.sh)" diff --git a/scripts/meta/resolve/pr/README.md b/scripts/meta/resolve/pr/README.md index 1e7b0c78bd..cbdd5c3bf3 100644 --- a/scripts/meta/resolve/pr/README.md +++ b/scripts/meta/resolve/pr/README.md @@ -7,6 +7,5 @@ Examples in this folder: - `scope.sh` resolves whether a Pull Request is agents-only, documentation-only, or full scope - `branch_prefix.sh` validates that the PR branch prefix matches the detected scope; see [branch.md](../../../../docs/contributing/artefact/git/branch.md) for the authoritative list of valid prefixes and their CI impact - `merge_ref.sh` resolves the merge ref used for forked PR workflows -- `subset_roles.py` parses the `roles:` block from the PR body when the `🧩 Subset` label is set, validates the ids against `roles/`, and emits the restricted whitelist; see the "Subset label" section in [pipeline.md](../../../../docs/contributing/artefact/git/pipeline.md) -These scripts keep PR scope detection, branch-prefix validation, fork merge-ref resolution, and subset-label parsing together in one place. +These scripts keep PR scope detection, branch-prefix validation, and fork merge-ref resolution together in one place. The `🧩 Subset` label's PR-body parser lives in the `cli.meta.ci.subset_roles` module; the dispatcher [pr_affected_roles.sh](../../../github/resolve/pr_affected_roles.sh) selects between it and the diff resolver. See the "Subset label" section in [pipeline.md](../../../../docs/contributing/artefact/git/pipeline.md). diff --git a/tests/integration/meta/workflows/test_subset_roles.py b/tests/integration/meta/workflows/test_subset_roles.py index 6677dfe230..a6eb36a682 100644 --- a/tests/integration/meta/workflows/test_subset_roles.py +++ b/tests/integration/meta/workflows/test_subset_roles.py @@ -1,5 +1,4 @@ import os -import shutil import subprocess import tempfile import unittest @@ -7,89 +6,83 @@ from pathlib import Path from . import PROJECT_ROOT -SCRIPT_REL = Path("scripts/meta/resolve/pr/subset_roles.py") -SCRIPT_PATH = PROJECT_ROOT / SCRIPT_REL +REAL_ROLES = ("web-app-nextcloud", "web-app-matomo") class TestSubsetRoles(unittest.TestCase): - """Exercises the '🧩 Subset' resolver. + """Exercises the cli.meta.ci.subset_roles resolver behind the '🧩 Subset' + label. The "no subset label" case (existing diff behaviour stays unchanged) is - covered by test_diff_affected_roles.py: this script only ever runs when + covered by test_diff_affected_roles.py: this module only ever runs when the label gates it in entry-pull-request-change.yml. """ - def _run(self, *, body: str, roles: tuple[str, ...] = ("web-app-foo",)): - tmp = Path(tempfile.mkdtemp(prefix="subset-roles-")) - self.addCleanup(shutil.rmtree, tmp, ignore_errors=True) + def _run(self, body: str): + with tempfile.TemporaryDirectory() as tmp: + output_file = Path(tmp) / "output.txt" + output_file.touch() - # Plant the real script at its expected relative path so REPO_ROOT - # (parents[4]) lands inside the temp tree, and create the roles/ dirs - # the validation checks against. - script_target = tmp / SCRIPT_REL - script_target.parent.mkdir(parents=True) - shutil.copy2(SCRIPT_PATH, script_target) - for role in roles: - (tmp / "roles" / role).mkdir(parents=True) + env = os.environ.copy() + env["PR_BODY"] = body + env["GITHUB_OUTPUT"] = str(output_file) - output_file = tmp / "output.txt" - output_file.touch() + result = subprocess.run( + ["python", "-m", "cli.meta.ci.subset_roles"], + cwd=PROJECT_ROOT, + env=env, + capture_output=True, + text=True, + check=False, + ) - env = os.environ.copy() - env["PR_BODY"] = body - env["GITHUB_OUTPUT"] = str(output_file) - - result = subprocess.run( - ["python", str(script_target)], - env=env, - capture_output=True, - text=True, - check=False, - ) - - outputs = {} - for line in output_file.read_text(encoding="utf-8").splitlines(): - if "=" in line: - key, value = line.split("=", 1) - outputs[key] = value - return result, outputs + outputs = {} + raw = output_file.read_text(encoding="utf-8") # nocheck: cache-read + for line in raw.splitlines(): + if "=" in line: + key, value = line.split("=", 1) + outputs[key] = value + return result, outputs def test_valid_roles_produce_whitelist(self): - body = "## Roles\n\n```yaml\nroles:\n - web-app-foo\n - web-app-bar\n```\n" - result, outputs = self._run(body=body, roles=("web-app-foo", "web-app-bar")) + body = ( + "## Roles\n\n```yaml\nroles:\n" + f" - {REAL_ROLES[0]}\n - {REAL_ROLES[1]}\n```\n" + ) + result, outputs = self._run(body) self.assertEqual(result.returncode, 0, result.stderr) - self.assertEqual(outputs["whitelist"], "web-app-foo web-app-bar") + self.assertEqual(outputs["whitelist"], " ".join(REAL_ROLES)) self.assertEqual(outputs["roles_only"], "true") def test_invalid_yaml_fails(self): - body = "```yaml\nroles:\n - web-app-foo\n bad: : :\n```\n" - result, outputs = self._run(body=body) + body = "```yaml\nroles:\n - web-app-nextcloud\n bad: : :\n```\n" + result, outputs = self._run(body) self.assertNotEqual(result.returncode, 0) self.assertIn("invalid YAML", result.stderr) self.assertNotIn("whitelist", outputs) def test_unknown_role_fails(self): - body = "```yaml\nroles:\n - web-app-foo\n - web-app-ghost\n```\n" - result, outputs = self._run(body=body, roles=("web-app-foo",)) + body = "```yaml\nroles:\n - web-app-nextcloud\n - web-app-ghost-xyz\n```\n" + result, outputs = self._run(body) self.assertNotEqual(result.returncode, 0) - self.assertIn("web-app-ghost", result.stderr) + self.assertIn("web-app-ghost-xyz", result.stderr) self.assertNotIn("whitelist", outputs) def test_path_traversal_id_is_rejected(self): body = "```yaml\nroles:\n - ../../etc\n```\n" - result, outputs = self._run(body=body) + result, outputs = self._run(body) self.assertNotEqual(result.returncode, 0) self.assertIn("invalid role id", result.stderr) self.assertNotIn("whitelist", outputs) def test_empty_role_list_fails(self): body = "```yaml\nroles:\n```\n" - result, _ = self._run(body=body) + result, _ = self._run(body) self.assertNotEqual(result.returncode, 0) self.assertIn("empty", result.stderr) def test_missing_block_fails(self): - result, _ = self._run(body="No machine-readable roles block here.\n") + result, _ = self._run("No machine-readable roles block here.\n") self.assertNotEqual(result.returncode, 0) self.assertIn("no fenced", result.stderr)