diff --git a/.github/CODEOWNERS b/.github/CODEOWNERS new file mode 100644 index 0000000000..4c14ef84f0 --- /dev/null +++ b/.github/CODEOWNERS @@ -0,0 +1,28 @@ +# Paths that need a hermes-agent-core approval before merge. This replaces the +# ``ci-reviewed`` label gate: GitHub requests the review and enforces it +# (ruleset: "Require review from Code Owners"), so CI no longer needs to +# classify these paths, check a label, or rerun jobs when one is added. +# +# Last match wins; every rule below has the same owner, so order is moot. +# pyproject.toml, uv.lock, flake.lock, package*.json are covered by the +# ruleset's required-reviewers rule and are intentionally not repeated here. + +# Workflows, composite actions, and this file: they decide what CI executes +# and what the autofix job pushes to main. +/.github/ @NousResearch/hermes-agent-core + +# eslint configs can define custom ``fix`` functions that run arbitrary code. +eslint.config.* @NousResearch/hermes-agent-core +/.prettierrc @NousResearch/hermes-agent-core + +# MCP catalog entries define local commands users later install into +# ``mcp_servers``; the installer decides how they are fetched and run. +/optional-mcps/ @NousResearch/hermes-agent-core +/hermes_cli/mcp_catalog.py @NousResearch/hermes-agent-core + +# Files that execute at pip-install or interpreter startup. +/setup.py @NousResearch/hermes-agent-core +/setup.cfg @NousResearch/hermes-agent-core +/sitecustomize.py @NousResearch/hermes-agent-core +/usercustomize.py @NousResearch/hermes-agent-core +*.pth @NousResearch/hermes-agent-core diff --git a/.github/actions/detect-changes/action.yml b/.github/actions/detect-changes/action.yml index d53b47fffd..3c17a72fcb 100644 --- a/.github/actions/detect-changes/action.yml +++ b/.github/actions/detect-changes/action.yml @@ -1,7 +1,7 @@ name: Detect affected areas description: >- Classify a PR's changed files into CI work lanes (python, frontend, site, - scan, deps, mcp_catalog) so the orchestrator can conditionally call only + scan, deps) so the orchestrator can conditionally call only the sub-workflows a PR can affect. Outputs are always "true" on push/dispatch events and fail open (everything "true") when the diff cannot be computed. @@ -66,15 +66,6 @@ outputs: rust: description: Run `cargo test` for the Tauri bootstrap installer. value: ${{ steps.classify.outputs.rust }} - 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 }} - ci_review_files: - description: JSON list of CI-sensitive files changed by the pull request. - value: ${{ steps.classify.outputs.ci_review_files }} runs: using: composite diff --git a/.github/workflows/ci.yaml b/.github/workflows/ci.yaml index adfb616188..ce2f204db1 100644 --- a/.github/workflows/ci.yaml +++ b/.github/workflows/ci.yaml @@ -76,13 +76,10 @@ jobs: desktop_updater: ${{ steps.gate-lanes.outputs.desktop_updater }} rust: ${{ steps.gate-lanes.outputs.rust }} docker_meta: ${{ steps.gate-lanes.outputs.docker_meta }} - mcp_catalog: ${{ steps.gate-lanes.outputs.mcp_catalog }} e2e: ${{ steps.gate-lanes.outputs.e2e }} e2e_upgrade: ${{ steps.gate-lanes.outputs.e2e_upgrade }} e2e_desktop_core: ${{ steps.gate-lanes.outputs.e2e_desktop_core }} e2e_desktop_update: ${{ steps.gate-lanes.outputs.e2e_desktop_update }} - ci_review: ${{ steps.classify.outputs.ci_review }} - ci_review_files: ${{ steps.classify.outputs.ci_review_files }} event_name: ${{ github.event_name }} steps: - uses: actions/checkout@f548e57e544e1ff5a4c46bf1e1b8685f8e4a348a # v7.0.1 @@ -93,7 +90,6 @@ jobs: github-token: ${{ github.token }} - name: Force all lanes on release # Overwrite the classifier outputs with 'true' when inputs.release. - # ci_review stays raw: it only ever feeds the PR review-label gate. id: gate-lanes env: RELEASE: ${{ inputs.release }} @@ -292,17 +288,6 @@ jobs: scan: ${{ needs.detect.outputs.scan == 'true' }} deps: ${{ needs.detect.outputs.deps == 'true' }} - review-labels: - name: Review label gate - needs: [detect, supply-chain] - if: always() && needs.detect.outputs.event_name == 'pull_request' && (needs.detect.outputs.ci_review == 'true' || needs.detect.outputs.mcp_catalog == 'true' || needs.supply-chain.outputs.critical_findings == 'true') - uses: ./.github/workflows/review-labels.yml - with: - ci_review: ${{ needs.detect.outputs.ci_review == 'true' }} - ci_review_files: ${{ needs.detect.outputs.ci_review_files }} - mcp_catalog: ${{ needs.detect.outputs.mcp_catalog == 'true' }} - supply_chain: ${{ needs.supply-chain.outputs.critical_findings == 'true' }} - # ───────────────────────────────────────────────────────────────────── # Gate: runs after everything. ``if: always()`` ensures it reports a # status even when some deps were skipped. @@ -310,7 +295,7 @@ jobs: # Non-release (PR/push): failure fails, skipped counts as success. # Release (inputs.release): strict — every required job must be # `success`. A skipped required lane fails the gate; only the PR-only - # jobs (history-check, lockfile-diff, supply-chain, review-labels) and + # jobs (history-check, lockfile-diff, supply-chain) and # the deferred Desktop E2E (e2e-desktop) may skip. The OSV scan's # findings are advisory, but its execution is required. # @@ -344,7 +329,6 @@ jobs: - profile-artifact-check - icons-freshness-check - supply-chain - - review-labels # OSV runs weekly against main (osv-scanner.yml schedule), not per PR: # every PR was reporting the same repo-wide baseline of pinned-dep CVEs # in its review comment, and the SARIF upload tripped GitHub's diff --git a/.github/workflows/label-rerun.yml b/.github/workflows/label-rerun.yml index d8d6fe7905..756193180b 100644 --- a/.github/workflows/label-rerun.yml +++ b/.github/workflows/label-rerun.yml @@ -1,19 +1,10 @@ name: Label rerun -# When the ``ci-reviewed`` label is added to a PR, rerun all failed jobs in -# the latest CI run. This re-evaluates ``review-labels`` (which now sees the -# label) and GitHub reruns the dependent ``all-checks-pass`` gate. -# # When the ``run-e2e`` label is added, rerun the head commit's CI, Docker and # Nix runs in full. Their ``detect`` job reads the PR's labels through the API # (a rerun replays the original event payload), so the rerun turns every slow # lane on (scripts/ci/classify_changes.py). # -# ``gh run rerun`` only works on a completed run. Thus this waits when the -# run is still in progress. The wait is now short. The two slowest jobs are -# the 40-minute comment poller and the 45-minute image build. Each one moved -# to its own workflow, so a CI run ends when its required jobs end. -# # The review comment updates without help. The poller in # ci-review-comment.yml watches the CI run through the API. It gets the # rerun results. @@ -26,82 +17,11 @@ permissions: actions: write pull-requests: read -# Keyed by label so one label's job does not cancel the other's. concurrency: group: label-rerun-${{ github.event.pull_request.number }}-${{ github.event.label.name }} cancel-in-progress: true jobs: - rerun-review-labels: - name: Rerun review-labels job - if: github.event.label.name == 'ci-reviewed' - runs-on: ubuntu-latest - timeout-minutes: 40 - steps: - - name: Wait for CI run to finish, then rerun failed jobs - env: - GH_TOKEN: ${{ secrets.GITHUB_TOKEN }} - REPO: ${{ github.repository }} - HEAD_SHA: ${{ github.event.pull_request.head.sha }} - PR: ${{ github.event.pull_request.number }} - run: | - set -uo pipefail - - # Find the latest CI run for this PR's head SHA. - RUN_INFO=$(gh run list \ - --repo "$REPO" \ - --commit "$HEAD_SHA" \ - --workflow ci.yaml \ - --limit 1 \ - --json databaseId,status \ - --jq '.[0] | "\(.databaseId) \(.status)"' 2>/dev/null || true) - - if [ -z "$RUN_INFO" ]; then - echo "No CI run found for this PR — nothing to rerun." - exit 0 - fi - - # Split "RUN_ID STATUS" into two vars. Read STATUS from RUN_INFO, - # not from the truncated RUN_ID. Both values came from the same - # var before, which made STATUS the run id. Thus the wait branch - # always ran. - RUN_ID="${RUN_INFO%% *}" - STATUS="${RUN_INFO##* }" - - echo "Latest CI run: $RUN_ID (status: $STATUS)" - - # If the run is still in progress, wait for it to finish. - # gh run rerun only works on completed runs — if we try while it's - # running, GitHub rejects with "cannot be rerun; This workflow is - # already running". - if [ "$STATUS" != "completed" ]; then - echo "Run is $STATUS — waiting for completion (this may take a while)..." - # gh run watch --exit-status exits non-zero if the run fails, - # which is expected (the label gate fails). Don't let that kill - # the workflow — we WANT to rerun failed jobs. - timeout 2100 gh run watch "$RUN_ID" --repo "$REPO" --interval 15 || true - - # Verify it's actually completed now. - STATUS=$(gh run view "$RUN_ID" --repo "$REPO" --json status --jq '.status' 2>/dev/null || echo "unknown") - if [ "$STATUS" != "completed" ]; then - echo "Run is still $STATUS after wait — giving up." - exit 0 - fi - fi - - # A push while we waited cancelled this run (that is the "completed" - # we saw). Rerunning it would rejoin the PR's CI concurrency group and - # cancel the new head's run, which already sees the label. - CURRENT_HEAD=$(gh pr view "$PR" --repo "$REPO" --json headRefOid --jq .headRefOid || true) - if [ "$CURRENT_HEAD" != "$HEAD_SHA" ]; then - echo "PR head moved to $CURRENT_HEAD; not rerunning superseded run $RUN_ID." - exit 0 - fi - - echo "Run completed. Rerunning all failed jobs..." - gh run rerun "$RUN_ID" --repo "$REPO" --failed || true - echo "Done. GitHub will rerun review-labels and all dependent jobs." - rerun-with-e2e: name: Rerun CI with every E2E lane if: github.event.label.name == 'run-e2e' diff --git a/.github/workflows/lint.yml b/.github/workflows/lint.yml index bfa41ef848..67d83c44af 100644 --- a/.github/workflows/lint.yml +++ b/.github/workflows/lint.yml @@ -8,8 +8,7 @@ name: Lint (ruff + ty) # Separate job so the advisory diff still runs even when enforcement # fails. # -# CI-sensitive file review was previously here as a ``ci-review`` job but -# has moved to ``review-labels.yml`` so it can be rerun independently. +# CI-sensitive file review is enforced by .github/CODEOWNERS, not a CI job. on: workflow_call: diff --git a/.github/workflows/lockfile-diff.yml b/.github/workflows/lockfile-diff.yml index bed10759ca..f7ccb41dd4 100644 --- a/.github/workflows/lockfile-diff.yml +++ b/.github/workflows/lockfile-diff.yml @@ -79,7 +79,7 @@ jobs: if [ "$CHANGED" = "true" ]; then CONTENT=$(cat /tmp/lockfile-diff.md | python3 -c "import sys,json; print(json.dumps(sys.stdin.read()))") - STATUS="[{\"source\":\"lockfile-diff\",\"results\":[{\"kind\":\"action_required\",\"title\":\"package-lock.json\",\"summary\":\"Locked npm dependency versions changed.\",\"detail\":${CONTENT},\"how_to_fix\":\"Add the \`ci-reviewed\` label after verifying the version changes are expected.\"}]}]" + STATUS="[{\"source\":\"lockfile-diff\",\"results\":[{\"kind\":\"action_required\",\"title\":\"package-lock.json\",\"summary\":\"Locked npm dependency versions changed.\",\"detail\":${CONTENT},\"how_to_fix\":\"Verify the version changes are expected. package-lock.json changes need a hermes-agent-core review.\"}]}]" else STATUS="[]" fi diff --git a/.github/workflows/review-labels.yml b/.github/workflows/review-labels.yml deleted file mode 100644 index b54cb1cc56..0000000000 --- a/.github/workflows/review-labels.yml +++ /dev/null @@ -1,123 +0,0 @@ -name: Review labels - -# Require explicit maintainer review when CI-sensitive files or the MCP -# catalog change. Previously this was split across two jobs in two -# workflows: ``ci-review`` in lint.yml (gated on ``ci_review``) and -# ``mcp-catalog-review`` in supply-chain-audit.yml (gated on -# ``mcp_catalog``). Both checked for their own label. -# -# Now consolidated: a single ``ci-reviewed`` label covers both. The -# comment sections tell the reviewer exactly what to verify per area, -# so one label is enough — the human reads the comment, not the label -# name. -# -# Outputs: -# ci_reviewed — "true" / "false" / "" (empty when neither lane ran) -# review_status — JSON array of status objects consumed by the review -# comment assembler. See scripts/ci/emit_review_status.py. - -on: - workflow_call: - inputs: - ci_review: - description: Whether CI-sensitive files (eslint config, workflows, actions) changed. - type: boolean - default: false - ci_review_files: - description: JSON list of CI-sensitive files changed by the pull request. - type: string - default: '[]' - mcp_catalog: - description: Whether the MCP catalog / installer changed. - type: boolean - default: false - supply_chain: - description: Whether the critical supply-chain scan found a risk requiring review. - type: boolean - default: false - outputs: - ci_reviewed: - description: Whether the ci-reviewed label is present. Empty when neither input was true. - value: ${{ jobs.check.outputs.ci_reviewed }} - review_status: - description: JSON array of status objects for the review comment assembler. - value: ${{ jobs.check.outputs.review_status }} - -permissions: - contents: read - pull-requests: read # read PR labels - -jobs: - check: - name: Review label gate - if: inputs.ci_review || inputs.mcp_catalog || inputs.supply_chain - runs-on: ubuntu-latest - timeout-minutes: 2 - outputs: - ci_reviewed: ${{ steps.label-check.outputs.ci_reviewed }} - review_status: ${{ steps.build-status.outputs.review_status }} - steps: - - name: Checkout code - uses: actions/checkout@f548e57e544e1ff5a4c46bf1e1b8685f8e4a348a # v7.0.1 - - - name: Check ci-reviewed label - id: label-check - env: - GH_TOKEN: ${{ secrets.GITHUB_TOKEN }} - REPO: ${{ github.repository }} - run: | - set -euo pipefail - PR="${{ github.event.pull_request.number }}" - LABELS=$(gh pr view "$PR" --repo "$REPO" --json labels --jq '.labels[].name' || true) - - if echo "$LABELS" | grep -Fxq 'ci-reviewed'; then - echo "ci-reviewed label present." - echo "ci_reviewed=true" >> "$GITHUB_OUTPUT" - else - echo "ci-reviewed label missing." - echo "ci_reviewed=false" >> "$GITHUB_OUTPUT" - fi - - - name: Build review_status JSON - id: build-status - env: - CI_REVIEW: ${{ inputs.ci_review }} - CI_REVIEW_FILES: ${{ inputs.ci_review_files }} - MCP_CATALOG: ${{ inputs.mcp_catalog }} - SUPPLY_CHAIN: ${{ inputs.supply_chain }} - LABEL_PRESENT: ${{ steps.label-check.outputs.ci_reviewed }} - REPO_URL: ${{ github.server_url }}/${{ github.repository }} - BASE_SHA: ${{ github.event.pull_request.base.sha }} - HEAD_SHA: ${{ github.event.pull_request.head.sha }} - run: | - set -euo pipefail - args=() - if [ "$CI_REVIEW" = "true" ]; then args+=(--ci-review); fi - args+=(--ci-review-files "$CI_REVIEW_FILES") - if [ "$MCP_CATALOG" = "true" ]; then args+=(--mcp-catalog); fi - if [ "$SUPPLY_CHAIN" = "true" ]; then args+=(--supply-chain); fi - if [ "$LABEL_PRESENT" = "true" ]; then args+=(--label-present); fi - - # Write to both $GITHUB_OUTPUT and review-status.json for the - # live comment poller to pick up as an artifact. - python3 scripts/ci/emit_review_status.py "${args[@]}" \ - --repo-url "$REPO_URL" --base-sha "$BASE_SHA" --head-sha "$HEAD_SHA" \ - --output "$GITHUB_OUTPUT" - grep '^review_status=' "$GITHUB_OUTPUT" > review-status.json - - - name: Upload review status artifact - if: always() && steps.build-status.outcome != 'skipped' - continue-on-error: true - uses: actions/upload-artifact@043fb46d1a93c77aae656e7c1c64a875d1fc6a0a # v7.0.1 - with: - name: review-status-review-labels - path: review-status.json - retention-days: 1 - overwrite: true - if-no-files-found: ignore - - - name: Fail on missing label - if: steps.label-check.outputs.ci_reviewed != 'true' - run: | - echo "::error::CI-sensitive changes require the ci-reviewed label. Add the label and re-run this check." - exit 1 diff --git a/.github/workflows/supply-chain-audit.yml b/.github/workflows/supply-chain-audit.yml index 15084aa53f..8f4cd85126 100644 --- a/.github/workflows/supply-chain-audit.yml +++ b/.github/workflows/supply-chain-audit.yml @@ -12,16 +12,16 @@ name: Supply Chain Audit # Path-gating is handled centrally by the ``ci.yml`` orchestrator's # ``detect`` job. The orchestrator passes ``scan`` / ``deps`` booleans as # inputs; this workflow's jobs gate on those inputs instead of re-computing -# the diff. MCP catalog review was previously here but has moved to -# ``review-labels.yml`` so it can be rerun independently. +# the diff. Files that run at install/startup (.pth, setup.py, ...) and the +# MCP catalog are not scanned here: .github/CODEOWNERS requires a maintainer +# review for them. # # Outputs: -# review_status — JSON array of status objects consumed by the review -# comment assembler (scripts/ci/assemble_review_comment.py). -# critical_findings — "true" when the narrow critical-pattern scan found -# something. The review-label gate consumes this and -# owns the action-required result, so adding -# ``ci-reviewed`` can heal the run on rerun. +# review_status — JSON array of status objects consumed by the review +# comment assembler (scripts/ci/assemble_review_comment.py). +# Each job (``scan``, ``dep-bounds``) emits its own +# array; an ``aggregate`` job merges them into the +# workflow-level output. on: workflow_call: @@ -42,9 +42,6 @@ on: review_status: description: JSON array of review status objects for the review comment assembler. value: ${{ jobs.aggregate.outputs.review_status }} - critical_findings: - description: Whether the critical-pattern scan found a risk requiring maintainer review. - value: ${{ jobs.aggregate.outputs.critical_findings }} permissions: pull-requests: write @@ -58,7 +55,6 @@ jobs: timeout-minutes: 15 outputs: review_status: ${{ steps.emit-status.outputs.review_status }} - critical_findings: ${{ steps.scan.outputs.found }} steps: - name: Checkout uses: actions/checkout@f548e57e544e1ff5a4c46bf1e1b8685f8e4a348a # v7.0.1 @@ -69,7 +65,6 @@ jobs: id: scan env: GH_TOKEN: ${{ github.token }} - CI_REVIEWED: ${{ contains(github.event.pull_request.labels.*.name, 'ci-reviewed') }} run: | set -euo pipefail @@ -84,22 +79,6 @@ jobs: FINDINGS="" - # --- .pth files (auto-execute on Python startup) --- - # The exact mechanism used in the litellm supply chain attack: - # https://github.com/BerriAI/litellm/issues/24512 - PTH_FILES=$(git diff --diff-filter=d --name-only "$BASE"..."$HEAD" | grep '\.pth$' || true) - if [ -n "$PTH_FILES" ]; then - FINDINGS="${FINDINGS} - ### 🚨 CRITICAL: .pth file added or modified - Python \`.pth\` files in \`site-packages/\` execute automatically when the interpreter starts — no import required. - - **Files:** - \`\`\` - ${PTH_FILES} - \`\`\` - " - fi - # --- base64 decode + exec/eval on the same line (the litellm attack pattern) --- B64_EXEC_HITS=$(echo "$DIFF" | grep -n '^+' | grep -iE 'base64\.(b64decode|decodebytes|urlsafe_b64decode)' | grep -iE 'exec\(|eval\(' | head -10 || true) if [ -n "$B64_EXEC_HITS" ]; then @@ -128,29 +107,6 @@ jobs: " fi - # --- Install-hook files (setup.py/sitecustomize/usercustomize/__init__.pth) --- - # These execute during pip install or interpreter startup. - # Anchored at repo root: only the top-level setup.py/setup.cfg run during - # `pip install`, and only top-level sitecustomize.py/usercustomize.py are - # auto-loaded by the interpreter via site.py. Any nested file with the - # same name (e.g. hermes_cli/setup.py — the CLI setup wizard) is unrelated - # and produced false positives that trained reviewers to ignore the scanner. - SETUP_HITS=$(git diff --diff-filter=d --name-only "$BASE"..."$HEAD" | grep -E '^(setup\.py|setup\.cfg|sitecustomize\.py|usercustomize\.py|__init__\.pth)$' || true) - # A maintainer-applied ci-reviewed label records the manual review - # required for intentional changes to an install hook. The scanner - # still blocks every unreviewed addition or modification. - if [ -n "$SETUP_HITS" ] && [ "$CI_REVIEWED" != "true" ]; then - FINDINGS="${FINDINGS} - ### 🚨 CRITICAL: Install-hook file added or modified - These files can execute code during package installation or interpreter startup. - - **Files:** - \`\`\` - ${SETUP_HITS} - \`\`\` - " - fi - if [ -n "$FINDINGS" ]; then echo "found=true" >> "$GITHUB_OUTPUT" echo "$FINDINGS" > /tmp/findings.md @@ -167,15 +123,33 @@ jobs: python3 - <<'PYEOF' import json, os - # The review-label gate renders and blocks critical findings. Keep - # this scan a fact-finder so adding ci-reviewed can rerun the gate - # without requiring the scanner itself to fail again. - status = [] + found = os.environ.get("FOUND", "") == "true" + + if found: + with open("/tmp/findings.md", encoding="utf-8") as f: + detail = f.read() + status = [{ + "source": "supply chain", + "results": [{ + "kind": "action_required", + "title": "Critical supply chain risk", + "summary": "Critical supply chain risk patterns detected in this PR.", + "detail": detail, + "how_to_fix": "Review the flagged code carefully. If it is a false positive, narrow the pattern in .github/workflows/supply-chain-audit.yml." + }] + }] + else: + status = [] with open(os.environ["GITHUB_OUTPUT"], "a", encoding="utf-8") as f: f.write(f"review_status={json.dumps(status)}\n") PYEOF + - name: Fail on critical findings + if: steps.scan.outputs.found == 'true' + run: | + echo "::error::CRITICAL supply chain risk patterns detected in this PR. See the review comment for details." + exit 1 dep-bounds: name: Check PyPI dependency upper bounds @@ -263,14 +237,12 @@ jobs: timeout-minutes: 15 outputs: review_status: ${{ steps.merge.outputs.review_status }} - critical_findings: ${{ steps.merge.outputs.critical_findings }} steps: - name: Merge review statuses id: merge env: SCAN_STATUS: ${{ needs.scan.outputs.review_status }} DEP_STATUS: ${{ needs.dep-bounds.outputs.review_status }} - CRITICAL_FINDINGS: ${{ needs.scan.outputs.critical_findings }} run: | python3 - <<'PYEOF' import json, os @@ -289,7 +261,6 @@ jobs: with open(os.environ["GITHUB_OUTPUT"], "a", encoding="utf-8") as f: f.write(f"review_status={json.dumps(merged)}\n") - f.write("critical_findings=" + os.environ.get("CRITICAL_FINDINGS", "false") + "\n") # Write review-status.json for the live comment poller artifact. with open("review-status.json", "w", encoding="utf-8") as f: diff --git a/scripts/ci/assemble_review_comment.py b/scripts/ci/assemble_review_comment.py index 5e8dc64ebd..fe799a6012 100644 --- a/scripts/ci/assemble_review_comment.py +++ b/scripts/ci/assemble_review_comment.py @@ -8,7 +8,7 @@ result objects:: [ { - "source": "review-label-gate", + "source": "supply chain", "results": [ {"kind": "action_required", "title": "...", "summary": "...", "how_to_fix": "..."}, @@ -170,7 +170,7 @@ def collect_failed_jobs( return [] # Pre-normalize exclude sources once: lowercase + hyphens→spaces, so - # "review-label-gate" matches "Review label gate / Review label gate". + # "supply-chain" matches "Supply-chain scan / ...". norm_sources = { src.lower().replace("-", " ") for src in (exclude_sources or set()) } @@ -378,7 +378,7 @@ def assemble( """Assemble the full comment body from all available inputs.""" items: list[ReviewItem] = [] - # 1. Structured statuses from workflow_call jobs (review-labels, etc.) + # 1. Structured statuses from workflow_call jobs (supply-chain, etc.) status_items, sources = collect_from_statuses(review_statuses_json) items.extend(status_items) diff --git a/scripts/ci/classify_changes.py b/scripts/ci/classify_changes.py index 04f64f5fcd..3c0d9d019f 100644 --- a/scripts/ci/classify_changes.py +++ b/scripts/ci/classify_changes.py @@ -39,7 +39,6 @@ Lanes: * ``rust`` — ``cargo test`` for the Tauri bootstrap installer. ``.rs`` lives under ``apps/``, so without this lane a Rust change matched ``frontend`` and only the TypeScript matrix ran. -* ``mcp_catalog`` — bundled MCP catalog / installer review. ``docker``, ``nix`` and the E2E lanes take most of the larger-runner minutes. An ordinary product change does not start them on a pull request. Every push @@ -118,27 +117,10 @@ _PY_RELEVANT_CONTRACT_FILES = { "plugin-catalog/README.md", } -# 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"} -# MCP catalog files that require explicit security review. -_MCP_CATALOG_PATHS = ("optional-mcps/",) -_MCP_CATALOG_FILES = {"hermes_cli/mcp_catalog.py"} - # Bootstrap installer: the POSIX shell installer, the dev-checkout wrapper # that carries the same pin fragment, and the Tauri app's non-Rust sources # (the .rs/Cargo files are the ``rust`` lane's job). Changes here get the @@ -293,10 +275,6 @@ def _is_scan(p: str) -> bool: return p.endswith(_SCAN_EXTS) or p in _SCAN_FILES -def _is_mcp_catalog(p: str) -> bool: - return p.startswith(_MCP_CATALOG_PATHS) or p in _MCP_CATALOG_FILES - - def _is_desktop_updater(p: str) -> bool: return ( p.startswith(_DESKTOP_UPDATER_PATHS) @@ -313,19 +291,6 @@ def _is_rust(p: str) -> bool: ) -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 ci_review_files(files: list[str]) -> list[str]: - """Return the CI-sensitive paths that need maintainer review.""" - return sorted({f.strip() for f in files if f.strip() and _is_ci_review(f.strip())}) - - def _slow_lanes(files: list[str]) -> dict[str, bool]: """The slow lanes a pull request with this diff runs; see ``_E2E_LANES``.""" lanes = { @@ -374,8 +339,6 @@ def classify(files: list[str], run_e2e: bool = False) -> dict[str, bool]: ), "desktop_updater": any(_is_desktop_updater(f) for f in files), "rust": any(_is_rust(f) 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), **{lane: run_e2e or on for lane, on in _slow_lanes(files).items()}, } if not files or any(f.startswith(".github/") for f in files): @@ -391,10 +354,7 @@ def classify(files: list[str], run_e2e: bool = False) -> dict[str, bool]: ret["bootstrap"] = True ret["desktop_updater"] = True ret["rust"] = True - ret["ci_review"] = True ret.update(dict.fromkeys(_slow_lanes([]), True)) - - # explicitly skip mcp catalog here. it's not needed unless those files are modified. return ret @@ -439,9 +399,8 @@ def pull_request_changed_files() -> list[str]: ``detect-changes`` calls ``repos/.../compare/base...head`` with raw SHAs. A fork force-push can 404 for ~30s until GitHub attaches the new head SHA - to the base repo, so the action fails open with an empty file list. That - forces ``ci_review=true`` and blocks the PR on a ``ci-reviewed`` label - even when no CI-sensitive file changed. + to the base repo, so the action fails open with an empty file list and + runs every lane even when the change is narrow. The pull-request files endpoint already knows the PR's files (it is how this action used to classify), so use it as a fallback on pull_request @@ -487,10 +446,7 @@ def main() -> int: ) files = recovered lanes = classify(files, run_e2e=RUN_E2E_LABEL in pull_request_labels()) - out = "\n".join([ - *(f"{key}={str(value).lower()}" for key, value in lanes.items()), - f"ci_review_files={json.dumps(ci_review_files(files))}", - ]) + out = "\n".join(f"{key}={str(value).lower()}" for key, value in lanes.items()) if dest := os.environ.get("GITHUB_OUTPUT"): with open(dest, "a", encoding="utf-8") as fh: fh.write(out + "\n") diff --git a/scripts/ci/emit_review_status.py b/scripts/ci/emit_review_status.py deleted file mode 100644 index 726d1f6b86..0000000000 --- a/scripts/ci/emit_review_status.py +++ /dev/null @@ -1,214 +0,0 @@ -#!/usr/bin/env python3 -"""Emit review_status JSON for the review-labels workflow. - -Builds a JSON array with one entry:: - - [ - { - "source": "review-label-gate", - "results": [ - {"kind": "action_required", "title": "...", "summary": "...", - "how_to_fix": "..."}, - {"kind": "info", "title": "...", "summary": "..."} - ] - } - ] - -The ``source`` field is the workflow name that declared the status; the -assembler uses it to exclude the corresponding job from the synthesized -❌ Error list (the job already has its own status section). - -The array can contain 0 to 3 results — one per lane that ran -(``ci_review``, ``mcp_catalog``, ``supply_chain``). When the ``ci-reviewed`` label is -present, the kind is ``info``; when missing, it's ``action_required`` -with the verification checklist. -""" - -from __future__ import annotations - -import argparse -import hashlib -import json -import sys -from urllib.parse import quote - -# The source identifier used for error-synthesis exclusion. This must -# match (as a normalized substring) the job name as it appears in the -# GitHub Actions API. The ci.yml job key is ``review-labels`` with -# ``name: Review label gate``, and the reusable workflow's job is also -# ``name: Review label gate``, so the API shows the job as -# "Review label gate / Review label gate". Normalizing "review-label-gate" -# (lowercase, hyphens→spaces) gives "review label gate", which is a -# substring of "review label gate / review label gate". -SOURCE = "review-label-gate" - - -def _ci_review_detail( - files_json: str, repo_url: str, base_sha: str, head_sha: str, -) -> str: - """Render links to the changed CI-sensitive files that triggered review.""" - try: - files = json.loads(files_json) - except (json.JSONDecodeError, TypeError): - return "" - if not isinstance(files, list) or not repo_url or not base_sha or not head_sha: - return "" - - links = [] - for path in files: - if not isinstance(path, str) or not path: - continue - label = path.replace("\\", "\\\\").replace("[", "\\[").replace("]", "\\]") - path_hash = hashlib.sha256(path.encode()).hexdigest() - url = ( - f"{repo_url}/compare/{quote(base_sha, safe='')}...{quote(head_sha, safe='')}" - f"#diff-{path_hash}" - ) - links.append(f"- [`{label}`]({url})") - return "**Sensitive files changed:**\n" + "\n".join(links) if links else "" - - -def build_results( - ci_review: bool, - mcp_catalog: bool, - supply_chain: bool, - label_present: bool, - ci_review_files: str = "[]", - repo_url: str = "", - base_sha: str = "", - head_sha: str = "", -) -> list[dict]: - """Build the list of result objects for this source.""" - results: list[dict] = [] - - if ci_review: - detail = _ci_review_detail(ci_review_files, repo_url, base_sha, head_sha) - if label_present: - result = { - "kind": "info", - "title": "CI-sensitive file review", - "summary": ( - "PR touches sensitive files, but the `ci-reviewed` label has been " - "added, approving them." - ), - } - else: - result = { - "kind": "action_required", - "title": "CI-sensitive file review", - "summary": ( - "This PR changes CI-sensitive files (eslint config, " - "workflow YAMLs, or composite actions). These influence " - "what the js-autofix job executes and pushes to main." - ), - "how_to_fix": ( - "Add the `ci-reviewed` label after verifying:\n" - "- no new eslint rules with custom `fix` functions that write outside linted paths,\n" - "- no workflow changes that widen permissions or remove guards,\n" - "- no composite action changes that alter what gets executed." - ), - } - if detail: - result["detail"] = detail - results.append(result) - - if mcp_catalog: - if label_present: - results.append({ - "kind": "debug", - "title": "MCP catalog security review", - "summary": "`ci-reviewed` label is present.", - }) - else: - results.append({ - "kind": "action_required", - "title": "MCP catalog security review", - "summary": ( - "This PR changes the bundled MCP catalog or MCP catalog " - "installer code. MCP entries can define local commands " - "that users later install into `mcp_servers`, so this " - "needs explicit maintainer review before merge." - ), - "how_to_fix": ( - "Add the `ci-reviewed` label after verifying:\n" - "- any new/changed `optional-mcps/**/manifest.yaml` command and args are expected,\n" - "- stdio transports do not use shell+egress/exfiltration payloads,\n" - "- git install refs are pinned and bootstrap commands are minimal,\n" - "- requested env vars/secrets match the upstream MCP's documented needs." - ), - }) - - if supply_chain and not label_present: - results.append({ - "kind": "action_required", - "title": "Critical supply chain risk", - "summary": "Critical supply chain risk patterns were detected in this PR.", - "how_to_fix": ( - "Review the flagged code carefully. If it is intentional, add the " - "`ci-reviewed` label to confirm maintainer review." - ), - }) - - return results - - -def build_statuses( - ci_review: bool, - mcp_catalog: bool, - supply_chain: bool, - label_present: bool, - ci_review_files: str = "[]", - repo_url: str = "", - base_sha: str = "", - head_sha: str = "", -) -> list[dict]: - """Build the full review_status array (one entry with a results list).""" - results = build_results( - ci_review, mcp_catalog, supply_chain, label_present, - ci_review_files, repo_url, base_sha, head_sha, - ) - if not results: - return [] - return [{"source": SOURCE, "results": results}] - - -def main() -> int: - parser = argparse.ArgumentParser(description=__doc__) - parser.add_argument("--ci-review", action="store_true", - help="Whether CI-sensitive files changed.") - parser.add_argument("--ci-review-files", default="[]", - help="JSON list of CI-sensitive files changed.") - parser.add_argument("--mcp-catalog", action="store_true", - help="Whether the MCP catalog / installer changed.") - parser.add_argument("--supply-chain", action="store_true", - help="Whether the critical supply-chain scanner found a risk.") - parser.add_argument("--label-present", action="store_true", - help="Whether the ci-reviewed label is present.") - parser.add_argument("--repo-url", default="", - help="Repository URL used for changed-file links.") - parser.add_argument("--base-sha", default="", - help="Pull request base SHA used for changed-file links.") - parser.add_argument("--head-sha", default="", - help="Pull request head SHA used for changed-file links.") - parser.add_argument("--output", default="-", - help="Output file ('-' for stdout, or a GITHUB_OUTPUT path).") - args = parser.parse_args() - - statuses = build_statuses( - args.ci_review, args.mcp_catalog, args.supply_chain, args.label_present, - args.ci_review_files, args.repo_url, args.base_sha, args.head_sha, - ) - json_str = json.dumps(statuses) - - if args.output == "-": - print(json_str) - else: - # GITHUB_OUTPUT format: key=value\n - with open(args.output, "a", encoding="utf-8") as f: - f.write(f"review_status={json_str}\n") - - return 0 - - -if __name__ == "__main__": - sys.exit(main()) diff --git a/scripts/ci/required_results.py b/scripts/ci/required_results.py index 8c6877dfd5..1ccfcb64ae 100644 --- a/scripts/ci/required_results.py +++ b/scripts/ci/required_results.py @@ -22,7 +22,7 @@ from typing import Any # Jobs that can never run on a release (push/tag) event, plus the deferred # Desktop E2E. These are the only skips a strict run tolerates. -PR_ONLY_JOBS = ("history-check", "lockfile-diff", "supply-chain", "review-labels") +PR_ONLY_JOBS = ("history-check", "lockfile-diff", "supply-chain") DEFERRED_JOBS = ("e2e-desktop",) EXCLUDED_JOBS = frozenset((*PR_ONLY_JOBS, *DEFERRED_JOBS)) diff --git a/tests/ci/test_classify_changes.py b/tests/ci/test_classify_changes.py index 96946f65be..42ef472221 100644 --- a/tests/ci/test_classify_changes.py +++ b/tests/ci/test_classify_changes.py @@ -24,7 +24,6 @@ if _spec is None or _spec.loader is None: _mod = importlib.util.module_from_spec(_spec) _spec.loader.exec_module(_mod) classify = _mod.classify -ci_review_files = _mod.ci_review_files pull_request_changed_files = _mod.pull_request_changed_files pull_request_labels = _mod.pull_request_labels main = _mod.main @@ -48,14 +47,12 @@ DEFAULT = { "bootstrap": True, "desktop_updater": True, "rust": True, - "mcp_catalog": False, - "ci_review": True, } SLOW_LANES = {"docker", "nix", "e2e", "e2e_upgrade", "e2e_desktop_core", "e2e_desktop_update"} -def _lanes(python=False, frontend=False, site=False, scan=False, deps=False, uv_lock=False, npm_lock=False, bootstrap=False, desktop_updater=False, rust=False, mcp_catalog=False, docker_meta=False, ci_review=False, python_prod=None, nix=False, docker=None, e2e=False, e2e_upgrade=False, e2e_desktop_core=False, e2e_desktop_update=False) -> dict[str, bool]: +def _lanes(python=False, frontend=False, site=False, scan=False, deps=False, uv_lock=False, npm_lock=False, bootstrap=False, desktop_updater=False, rust=False, docker_meta=False, python_prod=None, nix=False, docker=None, e2e=False, e2e_upgrade=False, e2e_desktop_core=False, e2e_desktop_update=False) -> dict[str, bool]: # python_prod tracks python except for tests-only diffs; default it to # python so the majority of cases don't need to spell it out. # @@ -83,8 +80,6 @@ def _lanes(python=False, frontend=False, site=False, scan=False, deps=False, uv_ "bootstrap": bootstrap, "desktop_updater": desktop_updater, "rust": rust, - "mcp_catalog": mcp_catalog, - "ci_review": ci_review, } @@ -256,44 +251,44 @@ CASES = { # Supply-chain lanes ".pth file → scan": (["evil.pth"], _lanes(python=True, scan=True)), "setup.py → scan": (["setup.py"], _lanes(python=True, scan=True, docker=True, nix=True, e2e_upgrade=True)), - "mcp catalog manifest → mcp_catalog": ( + # Files CODEOWNERS owns carry no lane of their own: they only route as code. + "mcp catalog manifest → python only": ( ["optional-mcps/foo/manifest.yaml"], - _lanes(python=True, mcp_catalog=True), + _lanes(python=True), ), - "mcp_catalog.py → mcp_catalog": ( + "mcp_catalog.py → python + scan": ( ["hermes_cli/mcp_catalog.py"], - _lanes(python=True, scan=True, mcp_catalog=True), + _lanes(python=True, scan=True), ), - # CI-sensitive files require explicit review label. - "eslint config → ci_review": ( + "eslint config → frontend": ( ["apps/desktop/eslint.config.mjs"], - _lanes(frontend=True, ci_review=True), + _lanes(frontend=True), ), - "shared eslint config → ci_review": ( + "shared eslint config → python": ( ["eslint.config.shared.mjs"], - _lanes(python=True, ci_review=True), + _lanes(python=True), ), - "ui-tui eslint config → ci_review": ( + "ui-tui eslint config → frontend": ( ["ui-tui/eslint.config.mjs"], - _lanes(frontend=True, ci_review=True), + _lanes(frontend=True), ), - "web eslint config → ci_review": ( + "web eslint config → frontend": ( ["web/eslint.config.js"], - _lanes(frontend=True, ci_review=True), + _lanes(frontend=True), ), - "shared package eslint config → ci_review": ( + "shared package eslint config → frontend": ( ["apps/shared/eslint.config.mjs"], - _lanes(frontend=True, ci_review=True), + _lanes(frontend=True), ), - "bootstrap-installer eslint config → ci_review": ( + "bootstrap-installer eslint config → frontend + bootstrap": ( ["apps/bootstrap-installer/eslint.config.mjs"], - _lanes(frontend=True, bootstrap=True, ci_review=True), + _lanes(frontend=True, bootstrap=True), ), - "prettier config → ci_review": ( + "prettier config → python": ( [".prettierrc"], - _lanes(python=True, ci_review=True), + _lanes(python=True), ), - "workflow yml → ci_review (also fail-open all)": ( + "workflow yml → fail-open all": ( [".github/workflows/typecheck.yml"], DEFAULT, ), @@ -311,12 +306,11 @@ CASES = { ["apps/bootstrap-installer/src-tauri/src/lib.rs"], _lanes(frontend=True, bootstrap=True, rust=True), ), - "composite action → ci_review (also fail-open all)": ( + "composite action → fail-open all": ( [".github/actions/retry/action.yml"], DEFAULT, ), - # Normal desktop source doesn't trigger ci_review. - "desktop src → no ci_review": ( + "desktop src → frontend only": ( ["apps/desktop/src/app.tsx"], _lanes(frontend=True), ), @@ -457,18 +451,6 @@ def _iter_if_expressions(job: object): yield cond -def test_ci_review_files_returns_only_sensitive_paths_sorted_and_unique(): - assert ci_review_files([ - "apps/desktop/src/app.tsx", - ".github/workflows/ci.yml", - "apps/desktop/eslint.config.mjs", - ".github/workflows/ci.yml", - ]) == [ - ".github/workflows/ci.yml", - "apps/desktop/eslint.config.mjs", - ] - - def _write_event(tmp_path, number: int | None = 88442) -> Path: payload = {"pull_request": {"number": number}} if number is not None else {} path = tmp_path / "event.json" @@ -521,8 +503,8 @@ def test_pull_request_changed_files_returns_empty_when_gh_fails(tmp_path, monkey assert pull_request_changed_files() == [] -def test_main_recovers_pr_files_instead_of_fail_open_ci_review(monkeypatch, capsys): - """A fork compare 404 must not demand ci-reviewed for a CLI-only install.""" +def test_main_recovers_pr_files_instead_of_fail_open(monkeypatch, capsys): + """A fork compare 404 must not turn every lane on for a CLI-only install.""" monkeypatch.setattr( _mod, "pull_request_changed_files", @@ -533,7 +515,7 @@ def test_main_recovers_pr_files_instead_of_fail_open_ci_review(monkeypatch, caps assert main() == 0 out = capsys.readouterr().out - assert "ci_review=false" in out + assert "frontend=false" in out assert "python=true" in out assert "python_prod=true" in out @@ -545,11 +527,11 @@ def test_main_still_fail_opens_when_recovery_is_empty(monkeypatch, capsys): assert main() == 0 out = capsys.readouterr().out - assert "ci_review=true" in out + assert "frontend=true" in out def test_main_reads_the_run_e2e_label(monkeypatch, capsys): - monkeypatch.setattr(_mod, "pull_request_labels", lambda: ["ci-reviewed", _mod.RUN_E2E_LABEL]) + monkeypatch.setattr(_mod, "pull_request_labels", lambda: [_mod.RUN_E2E_LABEL]) monkeypatch.setattr(sys, "stdin", io.StringIO("gateway/run.py\n")) monkeypatch.delenv("GITHUB_OUTPUT", raising=False) diff --git a/tests/ci/test_emit_review_status.py b/tests/ci/test_emit_review_status.py deleted file mode 100644 index 9ac1edfe0c..0000000000 --- a/tests/ci/test_emit_review_status.py +++ /dev/null @@ -1,50 +0,0 @@ -"""Tests for scripts/ci/emit_review_status.py.""" - -from __future__ import annotations - -import importlib.util -import sys -from pathlib import Path - -_PATH = Path(__file__).resolve().parents[2] / "scripts" / "ci" / "emit_review_status.py" -_spec = importlib.util.spec_from_file_location("emit_review_status", _PATH) -if _spec is None or _spec.loader is None: - raise ImportError("Failed to load emit_review_status.py") -_mod = importlib.util.module_from_spec(_spec) -sys.modules["emit_review_status"] = _mod -_spec.loader.exec_module(_mod) - - -def test_ci_review_status_links_to_each_sensitive_file_change(): - results = _mod.build_results( - ci_review=True, - mcp_catalog=False, - supply_chain=False, - label_present=False, - ci_review_files='[".github/workflows/ci.yml", "apps/desktop/eslint.config.mjs"]', - repo_url="https://github.com/nousresearch/hermes-agent", - base_sha="base456", - head_sha="abc123", - ) - - assert results[0]["detail"] == ( - "**Sensitive files changed:**\n" - "- [`.github/workflows/ci.yml`](https://github.com/nousresearch/hermes-agent/compare/base456...abc123#diff-b803fcb7f17ed9235f1e5cb1fcd2f5d3b2838429d4368ae4c57ce4436577f03f)\n" - "- [`apps/desktop/eslint.config.mjs`](https://github.com/nousresearch/hermes-agent/compare/base456...abc123#diff-a45471520795db6e46840d1ba2a82c1f8a2841039bd60fb50624488c5f192438)" - ) - - -def test_approved_ci_review_is_visible_info(): - results = _mod.build_results( - ci_review=True, - mcp_catalog=False, - supply_chain=False, - label_present=True, - ci_review_files='[".github/workflows/ci.yml"]', - repo_url="https://github.com/nousresearch/hermes-agent", - base_sha="base456", - head_sha="abc123", - ) - - assert len(results) == 1 - assert results[0]["kind"] == "info" diff --git a/tests/ci/test_required_results.py b/tests/ci/test_required_results.py index 813112455d..e14055f8bd 100644 --- a/tests/ci/test_required_results.py +++ b/tests/ci/test_required_results.py @@ -37,7 +37,7 @@ def test_status_policy(job, result, ordinary, excluded, release): def test_release_exclusion_policy(): assert required_results.EXCLUDED_JOBS == { - "history-check", "lockfile-diff", "supply-chain", "review-labels", "e2e-desktop", + "history-check", "lockfile-diff", "supply-chain", "e2e-desktop", }