diff --git a/.github/actions/detect-changes/action.yml b/.github/actions/detect-changes/action.yml index b2ab85be5bca..751190daf19b 100644 --- a/.github/actions/detect-changes/action.yml +++ b/.github/actions/detect-changes/action.yml @@ -27,6 +27,9 @@ outputs: mcp_catalog: description: Require MCP catalog security review label. value: ${{ steps.classify.outputs.mcp_catalog }} + ci_review: + description: Require CI-sensitive file review label. + value: ${{ steps.classify.outputs.ci_review }} runs: using: composite diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 7840bfdc361b..d5b4fab2dd86 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -43,6 +43,7 @@ jobs: deps: ${{ steps.classify.outputs.deps }} docker_meta: ${{ steps.classify.outputs.docker_meta }} mcp_catalog: ${{ steps.classify.outputs.mcp_catalog }} + ci_review: ${{ steps.classify.outputs.ci_review }} event_name: ${{ github.event_name }} steps: - uses: actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd # v6.0.2 @@ -65,10 +66,11 @@ jobs: lint: name: Python lints needs: detect - if: needs.detect.outputs.python == 'true' + if: needs.detect.outputs.python == 'true' || needs.detect.outputs.ci_review == 'true' uses: ./.github/workflows/lint.yml with: event_name: ${{ needs.detect.outputs.event_name }} + ci_review: ${{ needs.detect.outputs.ci_review == 'true' }} js-tests: name: JS & TS checks diff --git a/.github/workflows/lint.yml b/.github/workflows/lint.yml index beb3a07abaee..7f5a87f3b8dd 100644 --- a/.github/workflows/lint.yml +++ b/.github/workflows/lint.yml @@ -15,6 +15,10 @@ on: description: The event name from the calling orchestrator (pull_request or push). type: string required: true + ci_review: + description: Whether CI-sensitive files (eslint config, workflows, actions) changed and require a review label. + type: boolean + default: false permissions: contents: read @@ -158,3 +162,111 @@ jobs: - name: Run footgun checker run: python scripts/check-windows-footguns.py --all + + ci-review: + # Require explicit maintainer review when CI-sensitive files change: + # eslint config, workflow YAMLs, or composite actions. These files + # influence what code the desktop-autofix job executes and pushes to + # main, so a malicious PR could inject arbitrary code via a custom eslint + # rule's `fix` function. The label gate ensures a human reviews before + # merge. Mirrors the mcp-catalog-reviewed pattern in supply-chain-audit.yml. + name: CI-sensitive file review + if: inputs.event_name == 'pull_request' && inputs.ci_review + runs-on: ubuntu-latest + timeout-minutes: 2 + steps: + - name: Checkout code + uses: actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd # v6.0.2 + - name: Require ci-reviewed label + id: label-check + env: + GH_TOKEN: ${{ secrets.GITHUB_TOKEN }} + run: | + set -euo pipefail + PR="${{ github.event.pull_request.number }}" + LABELS=$(gh pr view "$PR" --json labels --jq '.labels[].name' || true) + if echo "$LABELS" | grep -Fxq 'ci-reviewed'; then + echo "reviewed=true" >> "$GITHUB_OUTPUT" + echo "ci-reviewed label present." + exit 0 + fi + echo "reviewed=false" >> "$GITHUB_OUTPUT" + + # On failure: find the bot's previous comment and edit it, or create + # a new one if none exists. Using an HTML comment marker so we can + # locate it reliably across runs without parsing the body text. + # Skipped on fork PRs — GITHUB_TOKEN is read-only there, so the API + # call would fail. The label gate still holds via the step below. + - name: Post or update review warning + if: steps.label-check.outputs.reviewed != 'true' && github.event.pull_request.head.repo.fork != true + env: + GH_TOKEN: ${{ secrets.GITHUB_TOKEN }} + run: | + set -euo pipefail + PR="${{ github.event.pull_request.number }}" + MARKER="" + BODY="${MARKER} + ## ⚠️ CI-sensitive file review required + + This PR changes CI-sensitive files (eslint config, workflow YAMLs, + or composite actions). These files influence what code the + desktop-autofix job executes and pushes to main. + + A maintainer should verify: + - no new eslint rules with custom \`fix\` functions that write outside linted paths, + - no workflow changes that widen permissions or remove guards, + - no composite action changes that alter what gets executed. + + After review, add the \`ci-reviewed\` label and re-run this check." + + # Find an existing comment with our marker. + COMMENT_ID=$(gh api \ + "repos/${{ github.repository }}/issues/${PR}/comments" \ + --paginate --jq ".[] | select(.body | contains(\"${MARKER}\")) | .id" \ + | head -1 || true) + + if [ -n "$COMMENT_ID" ]; then + gh api --method PATCH \ + "repos/${{ github.repository }}/issues/comments/${COMMENT_ID}" \ + -f body="$BODY" + else + gh pr comment "$PR" --body "$BODY" + fi + + # Fail the job when the label is missing — always runs (including + # fork PRs) so the security gate holds even when the comment step + # was skipped above. + - name: Fail on missing label + if: steps.label-check.outputs.reviewed != 'true' + run: | + echo "::error::CI-sensitive changes require the ci-reviewed label." + exit 1 + + # On success: if a previous warning comment exists, edit it to show + # the review passed so the PR doesn't have a stale ⚠️ sitting around. + # Skipped on fork PRs — no comment was ever posted to update. + - name: Update previous warning to passed + if: steps.label-check.outputs.reviewed == 'true' && github.event.pull_request.head.repo.fork != true + env: + GH_TOKEN: ${{ secrets.GITHUB_TOKEN }} + run: | + set -euo pipefail + PR="${{ github.event.pull_request.number }}" + MARKER="" + + # Find an existing comment with our marker. + COMMENT_ID=$(gh api \ + "repos/${{ github.repository }}/issues/${PR}/comments" \ + --paginate --jq ".[] | select(.body | contains(\"${MARKER}\")) | .id" \ + | head -1 || true) + + if [ -n "$COMMENT_ID" ]; then + BODY="${MARKER} + ## ✅ CI-sensitive file review passed + + The \`ci-reviewed\` label is present on this PR." + + gh api --method PATCH \ + "repos/${{ github.repository }}/issues/comments/${COMMENT_ID}" \ + -f body="$BODY" + fi diff --git a/scripts/ci/classify_changes.py b/scripts/ci/classify_changes.py index 00ed02d65895..1c3ea6694274 100644 --- a/scripts/ci/classify_changes.py +++ b/scripts/ci/classify_changes.py @@ -41,6 +41,19 @@ _SITE = ("website/", "skills/", "optional-skills/") # docs site + skill pages # Prose/frontend trees that can't touch Python. skills/ is excluded on purpose. _PY_SKIP = ("docs/", "website/") + _FRONTEND +# CI-sensitive files: eslint config, workflow files, composite actions. +# Changes here can influence what code the autofix job executes and pushes to +# main, so they require explicit maintainer review (ci-reviewed label). +# +# package.json is deliberately NOT listed here: npm scripts only execute on the +# unprivileged generate-patch runner (contents: read), never on the privileged +# apply-patch job. The two-job split means a malicious package.json script +# can't get push access — it runs on an ephemeral runner with zero write perms. +_CI_REVIEW_FILES = { + ".prettierrc", +} +_CI_REVIEW_PATHS = (".github/workflows/", ".github/actions/") + # Supply-chain scan: files that can execute code at install/import time. _SCAN_EXTS = (".py", ".pth") _SCAN_FILES = {"setup.cfg", "pyproject.toml"} @@ -67,6 +80,14 @@ def _is_mcp_catalog(p: str) -> bool: return p.startswith(_MCP_CATALOG_PATHS) or p in _MCP_CATALOG_FILES +def _is_ci_review(p: str) -> bool: + if p in _CI_REVIEW_FILES or p.startswith(_CI_REVIEW_PATHS): + return True + # Any eslint config file at any path — eslint configs can define custom + # fix functions that execute arbitrary code, so they all require review. + return os.path.basename(p).startswith("eslint.config.") + + def classify(files: list[str]) -> dict[str, bool]: """Map changed paths to ``{lane: should_run}``.""" files = [f.strip() for f in files if f.strip()] @@ -78,6 +99,7 @@ def classify(files: list[str]) -> dict[str, bool]: "scan": any(_is_scan(f) for f in files), "deps": any(f == "pyproject.toml" for f in files), "mcp_catalog": any(_is_mcp_catalog(f) for f in files), + "ci_review": any(_is_ci_review(f) for f in files), } if not files or any(f.startswith(".github/") for f in files): ret["python"] = True @@ -86,6 +108,7 @@ def classify(files: list[str]) -> dict[str, bool]: ret["site"] = True ret["scan"] = True ret["deps"] = True + ret["ci_review"] = True # explicitly skip mcp catalog here. it's not needed unless those files are modified. return ret diff --git a/tests/ci/test_classify_changes.py b/tests/ci/test_classify_changes.py index e1db0ccf20dd..f1772d895115 100644 --- a/tests/ci/test_classify_changes.py +++ b/tests/ci/test_classify_changes.py @@ -28,10 +28,11 @@ DEFAULT = { "scan": True, "deps": True, "mcp_catalog": False, + "ci_review": True, } -def _lanes(python=False, frontend=False, site=False, scan=False, deps=False, mcp_catalog=False, docker_meta=False) -> dict[str, bool]: +def _lanes(python=False, frontend=False, site=False, scan=False, deps=False, mcp_catalog=False, docker_meta=False, ci_review=False) -> dict[str, bool]: return { "python": python, "frontend": frontend, @@ -40,6 +41,7 @@ def _lanes(python=False, frontend=False, site=False, scan=False, deps=False, mcp "scan": scan, "deps": deps, "mcp_catalog": mcp_catalog, + "ci_review": ci_review, } @@ -72,6 +74,48 @@ CASES = { ["hermes_cli/mcp_catalog.py"], _lanes(python=True, scan=True, mcp_catalog=True), ), + # CI-sensitive files require explicit review label. + "eslint config → ci_review": ( + ["apps/desktop/eslint.config.mjs"], + _lanes(frontend=True, ci_review=True), + ), + "shared eslint config → ci_review": ( + ["eslint.config.shared.mjs"], + _lanes(python=True, ci_review=True), + ), + "ui-tui eslint config → ci_review": ( + ["ui-tui/eslint.config.mjs"], + _lanes(frontend=True, ci_review=True), + ), + "web eslint config → ci_review": ( + ["web/eslint.config.js"], + _lanes(frontend=True, ci_review=True), + ), + "shared package eslint config → ci_review": ( + ["apps/shared/eslint.config.mjs"], + _lanes(frontend=True, ci_review=True), + ), + "bootstrap-installer eslint config → ci_review": ( + ["apps/bootstrap-installer/eslint.config.mjs"], + _lanes(frontend=True, ci_review=True), + ), + "prettier config → ci_review": ( + [".prettierrc"], + _lanes(python=True, ci_review=True), + ), + "workflow yml → ci_review (also fail-open all)": ( + [".github/workflows/typecheck.yml"], + DEFAULT, + ), + "composite action → ci_review (also fail-open all)": ( + [".github/actions/retry/action.yml"], + DEFAULT, + ), + # Normal desktop source doesn't trigger ci_review. + "desktop src → no ci_review": ( + ["apps/desktop/src/app.tsx"], + _lanes(frontend=True), + ), # Fail open: CI-config / empty / blank diffs run everything. ".github change → all": ([".github/workflows/tests.yml"], DEFAULT), "action change → all": ([".github/actions/detect-changes/action.yml"], DEFAULT),