From 957ea640de2bda785205976a3907073016badc10 Mon Sep 17 00:00:00 2001 From: ethernet Date: Wed, 22 Jul 2026 22:15:23 -0400 Subject: [PATCH] fix(ci): publish inline E2E evidence (#69699) * fix(ci): publish inline E2E evidence Upload bounded screenshot evidence from E2E, then publish validated images from a trusted workflow_run job to commit-pinned branches in the evidence repo. Wait briefly for the live CI review comment marker before publishing, so GitHub's read-after-write delay cannot leave an orphaned evidence branch. * fix(ci): isolate privileged credentials from PR jobs Keep App private keys and Docker Hub credentials out of PR-controlled workflows. Use protected environments for trusted publishing and a public repository variable for the App client ID. * fix(ci): attach E2E evidence with restricted bot session Replace the App-backed evidence repository publisher with gh-image uploads from a dedicated bot session in the gh-image environment. * fix(ci): publish validated E2E evidence from forks Let the trusted default-branch publisher handle bounded, validated evidence artifacts from fork PR CI without checking out or executing fork code. --- .github/actions/get-app-token/action.yml | 26 +- .github/workflows/ci.yml | 43 +-- .github/workflows/deploy-site.yml | 2 +- .github/workflows/docker.yml | 118 +++++--- .github/workflows/e2e-desktop.yml | 13 + .github/workflows/js-autofix.yml | 3 +- .github/workflows/publish-e2e-evidence.yml | 69 +++++ .github/workflows/skills-index-freshness.yml | 3 +- .github/workflows/skills-index.yml | 6 +- .github/workflows/supply-chain-audit.yml | 9 +- scripts/ci/publish_e2e_evidence.py | 283 +++++++++++++++++++ tests/ci/test_publish_e2e_evidence.py | 136 +++++++++ 12 files changed, 611 insertions(+), 100 deletions(-) create mode 100644 .github/workflows/publish-e2e-evidence.yml create mode 100644 scripts/ci/publish_e2e_evidence.py create mode 100644 tests/ci/test_publish_e2e_evidence.py diff --git a/.github/actions/get-app-token/action.yml b/.github/actions/get-app-token/action.yml index 611533f28337..2aaf303ab2de 100644 --- a/.github/actions/get-app-token/action.yml +++ b/.github/actions/get-app-token/action.yml @@ -5,24 +5,32 @@ description: >- 5,000 req/hr per installation (vs 1,000 for the default GITHUB_TOKEN) and are scoped to the App's installation permissions, not a user account. - Falls back to the built-in GITHUB_TOKEN when APP_CLIENT_ID is not set — - this happens on fork PRs where repo secrets are unavailable. The fallback - ensures classification, timings, and review comments still work on - forks (with the lower GITHUB_TOKEN rate limit). + Callers must source App credentials from a protected, main-only environment. + Never pass an App private key to a pull_request job, a local action, or a + reusable workflow resolved from an untrusted PR ref. The fallback keeps a + trusted caller functional when its protected environment is misconfigured. - Composite actions cannot access the secrets context directly, so the - calling workflow must pass secrets.APP_CLIENT_ID and secrets.APP_PRIVATE_KEY - as inputs. When both are empty (fork PRs), the fallback fires. + Composite actions cannot access contexts directly, so callers pass the + public vars.APP_CLIENT_ID and protected secrets.APP_PRIVATE_KEY as inputs. + When the private key is empty, the fallback fires. inputs: client-id: - description: GitHub App Client ID. Pass secrets.APP_CLIENT_ID from the calling workflow. + description: GitHub App Client ID. Pass vars.APP_CLIENT_ID from the calling workflow. required: false default: '' private-key: description: GitHub App private key PEM. Pass secrets.APP_PRIVATE_KEY from the calling workflow. required: false default: '' + owner: + description: GitHub App installation owner. Empty scopes the token to the current repository. + required: false + default: '' + repositories: + description: Comma- or newline-separated repositories to scope within the installation owner. + required: false + default: '' outputs: token: @@ -51,6 +59,8 @@ runs: with: client-id: ${{ inputs.client-id }} private-key: ${{ inputs.private-key }} + owner: ${{ inputs.owner }} + repositories: ${{ inputs.repositories }} - name: Fall back to GITHUB_TOKEN id: fallback diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 0aa540ed8bee..caa61bb773de 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -9,6 +9,10 @@ name: CI # definitions, matrices, and concurrency settings. They no longer have # ``push:`` / ``pull_request:`` triggers of their own — everything flows # through this file. +# +# SECURITY: this workflow runs PR-controlled actions, workflows, and code. +# Do not add ``secrets: inherit`` or GitHub App credentials here. Trusted +# main-only automation uses protected environments in its own workflows. on: pull_request: @@ -50,19 +54,11 @@ jobs: event_name: ${{ github.event_name }} steps: - uses: actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd # v6.0.2 - - name: Get GitHub App token - id: app-token - uses: ./.github/actions/get-app-token - with: - client-id: ${{ secrets.APP_CLIENT_ID }} - private-key: ${{ secrets.APP_PRIVATE_KEY }} - name: Detect affected areas id: classify uses: ./.github/actions/detect-changes with: - # The get-app-token composite action falls back to GITHUB_TOKEN - # on fork PRs where APP_ID is unavailable. - github-token: ${{ steps.app-token.outputs.token }} + github-token: ${{ github.token }} # ───────────────────────────────────────────────────────────────────── # Lane-gated sub-workflows. Each runs in parallel after detect finishes. @@ -75,7 +71,6 @@ jobs: uses: ./.github/workflows/tests.yml with: slice_count: 8 - secrets: inherit lint: name: Python lints @@ -84,14 +79,12 @@ jobs: uses: ./.github/workflows/lint.yml with: event_name: ${{ needs.detect.outputs.event_name }} - secrets: inherit js-tests: name: JS & TS checks needs: detect if: needs.detect.outputs.frontend == 'true' uses: ./.github/workflows/js-tests.yml - secrets: inherit e2e-desktop: name: Desktop E2E @@ -104,48 +97,41 @@ jobs: needs: detect if: needs.detect.outputs.site == 'true' uses: ./.github/workflows/docs-site-checks.yml - secrets: inherit history-check: name: Deny unrelated histories needs: detect if: needs.detect.outputs.event_name == 'pull_request' uses: ./.github/workflows/history-check.yml - secrets: inherit contributor-check: name: Check contributors needs: detect if: needs.detect.outputs.python == 'true' uses: ./.github/workflows/contributor-check.yml - secrets: inherit uv-lockfile: name: Check uv.lock needs: detect uses: ./.github/workflows/uv-lockfile-check.yml - secrets: inherit lockfile-diff: name: package-lock.json diff needs: detect if: needs.detect.outputs.event_name == 'pull_request' && needs.detect.outputs.npm_lock == 'true' uses: ./.github/workflows/lockfile-diff.yml - secrets: inherit docker-lint: name: Lint Docker scripts needs: detect if: needs.detect.outputs.docker_meta == 'true' uses: ./.github/workflows/docker-lint.yml - secrets: inherit docker: name: Build&Test Docker image needs: detect if: needs.detect.outputs.python == 'true' || needs.detect.outputs.frontend == 'true' || needs.detect.outputs.docker_meta == 'true' uses: ./.github/workflows/docker.yml - secrets: inherit supply-chain: name: Supply-chain scan @@ -167,12 +153,10 @@ jobs: 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' }} - secrets: inherit osv-scanner: name: OSV scan uses: ./.github/workflows/osv-scanner.yml - secrets: inherit # ───────────────────────────────────────────────────────────────────── # Live-updating PR review comment. @@ -195,6 +179,9 @@ jobs: steps: - name: Checkout code uses: actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd # v6.0.2 + with: + ref: ${{ github.event.repository.default_branch }} + persist-credentials: false - name: Run live comment poller env: @@ -328,13 +315,6 @@ jobs: - name: Checkout code uses: actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd # v6.0.2 - - name: Get GitHub App token - id: app-token - uses: ./.github/actions/get-app-token - with: - client-id: ${{ secrets.APP_CLIENT_ID }} - private-key: ${{ secrets.APP_PRIVATE_KEY }} - - name: Restore baseline cache (PR only) if: github.event_name == 'pull_request' uses: actions/cache/restore@27d5ce7f107fe9357f9df03efb73ab90386fccae # v5.0.5 @@ -348,12 +328,7 @@ jobs: - name: Collect timings and generate report env: - # Forks get no repo secrets (AUTOFIX_BOT_PAT is empty); fall back to - # the built-in read-only token so the timings API read still works - # there instead of hard-failing this advisory job on every fork PR. - # The get-app-token composite action falls back to GITHUB_TOKEN - # on fork PRs where APP_ID is unavailable. - GITHUB_TOKEN: ${{ steps.app-token.outputs.token }} + GITHUB_TOKEN: ${{ github.token }} run: | python3 scripts/ci/timings_report.py \ --baseline ci-timings-baseline.json \ diff --git a/.github/workflows/deploy-site.yml b/.github/workflows/deploy-site.yml index fd09205a055c..3ac2c4741f89 100644 --- a/.github/workflows/deploy-site.yml +++ b/.github/workflows/deploy-site.yml @@ -60,7 +60,7 @@ jobs: id: app-token uses: ./.github/actions/get-app-token with: - client-id: ${{ secrets.APP_CLIENT_ID }} + client-id: ${{ vars.APP_CLIENT_ID }} private-key: ${{ secrets.APP_PRIVATE_KEY }} - uses: actions/setup-node@49933ea5288caeca8642d1e84afbd3f7d6820020 # v4 diff --git a/.github/workflows/docker.yml b/.github/workflows/docker.yml index f500aca99537..16165a9e64b6 100644 --- a/.github/workflows/docker.yml +++ b/.github/workflows/docker.yml @@ -20,7 +20,9 @@ env: IMAGE_NAME: nousresearch/hermes-agent jobs: - # Build, test, and optionally push the image for each architecture. + # Build and test the image for each architecture. This job runs PR code, + # so it must remain secret-free. Publishing happens in the separate, + # protected publish job after these tests pass. build: if: github.repository == 'NousResearch/hermes-agent' strategy: @@ -62,49 +64,6 @@ jobs: cache-from: ${{ matrix.cache-from }} cache-to: ${{ (github.event_name != 'pull_request') && matrix.cache-to || '' }} - - name: Log in to Docker Hub - if: github.event_name == 'push' && github.ref == 'refs/heads/main' || github.event_name == 'release' - uses: docker/login-action@4907a6ddec9925e35a0a9e82d7399ccc52663121 # v4.1.0 - with: - username: ${{ secrets.DOCKERHUB_USERNAME }} - password: ${{ secrets.DOCKERHUB_TOKEN }} - - # Push by digest only (no tag). The merge job assembles the - # tagged manifest list. `push-by-digest=true` is docker's recommended - # pattern for multi-runner multi-platform builds. - - name: Push ${{ matrix.arch }} by digest - id: push - if: github.event_name == 'push' && github.ref == 'refs/heads/main' || github.event_name == 'release' - uses: docker/build-push-action@bcafcacb16a39f128d818304e6c9c0c18556b85f # v7.1.0 - with: - context: . - file: Dockerfile - platforms: ${{ matrix.platform }} - labels: | - org.opencontainers.image.revision=${{ github.sha }} - build-args: | - HERMES_GIT_SHA=${{ github.sha }} - outputs: type=image,name=${{ env.IMAGE_NAME }},push-by-digest=true,name-canonical=true,push=true - cache-from: ${{ matrix.cache-from }} - cache-to: ${{ matrix.cache-to }} - - # Write the digest to a file and upload it as an artifact so the - # merge job can stitch both per-arch digests into a manifest list. - - name: Export digest - if: github.event_name == 'push' && github.ref == 'refs/heads/main' || github.event_name == 'release' - run: | - mkdir -p /tmp/digests - digest="${{ steps.push.outputs.digest }}" - touch "/tmp/digests/${digest#sha256:}" - - - name: Upload digest artifact - if: github.event_name == 'push' && github.ref == 'refs/heads/main' || github.event_name == 'release' - uses: actions/upload-artifact@043fb46d1a93c77aae656e7c1c64a875d1fc6a0a # v7 - with: - name: digest-${{ matrix.arch }} - path: /tmp/digests/* - if-no-files-found: error - retention-days: 1 # Run the docker-integration test suite against the freshly-built # image already loaded into the local daemon (`:test`). @@ -147,6 +106,74 @@ jobs: run: | scripts/run_tests.sh tests/docker/ --file-timeout 600 + # --------------------------------------------------------------------------- + # Rebuild and push each architecture only after the unprivileged build/test + # matrix passes. This job is the sole Docker Hub credential boundary. + # --------------------------------------------------------------------------- + publish: + if: github.repository == 'NousResearch/hermes-agent' && (github.event_name == 'push' && github.ref == 'refs/heads/main' || github.event_name == 'release') + needs: [build] + environment: container-publish + strategy: + fail-fast: false + matrix: + include: + - arch: amd64 + runner: ubuntu-latest + platform: linux/amd64 + cache-from: type=gha,scope=docker-amd64 + cache-to: type=gha,mode=max,scope=docker-amd64 + - arch: arm64 + runner: ubuntu-24.04-arm + platform: linux/arm64 + cache-from: type=gha,scope=docker-arm64 + cache-to: type=gha,mode=max,scope=docker-arm64 + runs-on: ${{ matrix.runner }} + timeout-minutes: 30 + steps: + - name: Checkout trusted source + uses: actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd # v6.0.2 + + - name: Set up Docker Buildx + uses: docker/setup-buildx-action@8d2750c68a42422c14e847fe6c8ac0403b4cbd6f # v3 + + - name: Log in to Docker Hub + uses: docker/login-action@4907a6ddec9925e35a0a9e82d7399ccc52663121 # v4.1.0 + with: + username: ${{ secrets.DOCKERHUB_USERNAME }} + password: ${{ secrets.DOCKERHUB_TOKEN }} + + # Push by digest only (no tag). The merge job assembles the tagged + # manifest list after both architecture publishers complete. + - name: Push ${{ matrix.arch }} by digest + id: push + uses: docker/build-push-action@bcafcacb16a39f128d818304e6c9c0c18556b85f # v7.1.0 + with: + context: . + file: Dockerfile + platforms: ${{ matrix.platform }} + labels: | + org.opencontainers.image.revision=${{ github.sha }} + build-args: | + HERMES_GIT_SHA=${{ github.sha }} + outputs: type=image,name=${{ env.IMAGE_NAME }},push-by-digest=true,name-canonical=true,push=true + cache-from: ${{ matrix.cache-from }} + cache-to: ${{ matrix.cache-to }} + + - name: Export digest + run: | + mkdir -p /tmp/digests + digest="${{ steps.push.outputs.digest }}" + touch "/tmp/digests/${digest#sha256:}" + + - name: Upload digest artifact + uses: actions/upload-artifact@043fb46d1a93c77aae656e7c1c64a875d1fc6a0a # v7 + with: + name: digest-${{ matrix.arch }} + path: /tmp/digests/* + if-no-files-found: error + retention-days: 1 + # --------------------------------------------------------------------------- # Stitch both per-arch digests into a single tagged multi-arch manifest. # This is a registry-side operation — no building, no layer re-push — @@ -158,8 +185,9 @@ jobs: merge: if: github.repository == 'NousResearch/hermes-agent' && (github.event_name == 'push' && github.ref == 'refs/heads/main' || github.event_name == 'release') runs-on: ubuntu-latest - needs: [build] + needs: [publish] timeout-minutes: 10 + environment: container-publish steps: - name: Download digests uses: actions/download-artifact@d3f86a106a0bac45b974a628896c90dbdf5c8093 # v4 diff --git a/.github/workflows/e2e-desktop.yml b/.github/workflows/e2e-desktop.yml index 55af27e24c2e..e9131c725224 100644 --- a/.github/workflows/e2e-desktop.yml +++ b/.github/workflows/e2e-desktop.yml @@ -171,6 +171,19 @@ jobs: echo '__E2E_REVIEW_STATUS__' } >> "$GITHUB_OUTPUT" + # The trusted workflow_run publisher consumes only this flat, bounded + # artifact. It turns selected images into GitHub attachment URLs; it + # never checks out or runs this PR's code. + - name: Upload inline E2E evidence + if: always() && github.ref_name != 'main' + uses: actions/upload-artifact@043fb46d1a93c77aae656e7c1c64a875d1fc6a0a # v7.0.1 + with: + name: e2e-evidence-${{ github.sha }} + path: /tmp/e2e-evidence + retention-days: 14 + overwrite: true + if-no-files-found: error + # ── Generate step summary with visual diff info ─────────────────── # Parse the JSON report + scan for diff images, then post a summary # to the GitHub Actions step output so reviewers can see what changed diff --git a/.github/workflows/js-autofix.yml b/.github/workflows/js-autofix.yml index 8fb0460bc056..2dfbe7e0b17a 100644 --- a/.github/workflows/js-autofix.yml +++ b/.github/workflows/js-autofix.yml @@ -122,6 +122,7 @@ jobs: if: needs.generate-patch.outputs.has-fixes == 'true' runs-on: ubuntu-latest timeout-minutes: 15 + environment: trusted-automation permissions: contents: write # needed to push to bot/js-autofix pull-requests: write # needed for PR creation + auto-merge @@ -132,7 +133,7 @@ jobs: id: app-token uses: ./.github/actions/get-app-token with: - client-id: ${{ secrets.APP_CLIENT_ID }} + client-id: ${{ vars.APP_CLIENT_ID }} private-key: ${{ secrets.APP_PRIVATE_KEY }} - name: Download patch diff --git a/.github/workflows/publish-e2e-evidence.yml b/.github/workflows/publish-e2e-evidence.yml new file mode 100644 index 000000000000..23c471ce2642 --- /dev/null +++ b/.github/workflows/publish-e2e-evidence.yml @@ -0,0 +1,69 @@ +name: Publish E2E evidence + +# This runs only from the default branch after CI completes. It intentionally +# checks out main, never the PR ref, and treats the downloaded artifact as +# untrusted input before uploading validated GitHub attachments. +on: + workflow_run: + workflows: [CI] + types: [completed] + +permissions: + actions: read + contents: read + pull-requests: write + +concurrency: + group: publish-e2e-evidence-${{ github.event.workflow_run.id }} + cancel-in-progress: false + +jobs: + publish: + name: Publish inline E2E evidence + if: github.event.workflow_run.event == 'pull_request' + runs-on: ubuntu-latest + timeout-minutes: 10 + environment: gh-image + steps: + - name: Check out trusted publisher + uses: actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd # v6.0.2 + with: + ref: ${{ github.event.repository.default_branch }} + persist-credentials: false + + # v1.2.0 resolves to 44f4b93ecbbe22de6c45fa2f62f519aee564ca8c. + - name: Install gh-image + run: gh extension install drogers0/gh-image --pin v1.2.0 + + - name: Download and attach evidence + env: + GH_TOKEN: ${{ github.token }} + GITHUB_TOKEN: ${{ github.token }} + GH_SESSION_TOKEN: ${{ secrets.GH_IMAGE_SESSION_TOKEN }} + SOURCE_REPO: ${{ github.repository }} + SOURCE_RUN_ID: ${{ github.event.workflow_run.id }} + run: | + set -euo pipefail + + PR_NUMBER=$(gh api "repos/$SOURCE_REPO/actions/runs/$SOURCE_RUN_ID" --jq '.pull_requests[0].number // empty') + if [ -z "$PR_NUMBER" ]; then + echo "No pull request is associated with CI run $SOURCE_RUN_ID." + exit 0 + fi + + ARTIFACT_NAME=$(gh api "repos/$SOURCE_REPO/actions/runs/$SOURCE_RUN_ID/artifacts" \ + --jq '.artifacts[] | select(.expired == false and (.name | startswith("e2e-evidence-"))) | .name' \ + | python3 -c 'import sys; print(next(iter(sys.stdin), "").strip())') + if [ -z "$ARTIFACT_NAME" ]; then + echo "No E2E evidence artifact was produced for CI run $SOURCE_RUN_ID." + exit 0 + fi + + EVIDENCE_DIR="$RUNNER_TEMP/e2e-evidence" + mkdir -p "$EVIDENCE_DIR" + gh run download "$SOURCE_RUN_ID" --repo "$SOURCE_REPO" --name "$ARTIFACT_NAME" --dir "$EVIDENCE_DIR" + + python3 scripts/ci/publish_e2e_evidence.py \ + --evidence-dir "$EVIDENCE_DIR" \ + --source-repo "$SOURCE_REPO" \ + --pr-number "$PR_NUMBER" diff --git a/.github/workflows/skills-index-freshness.yml b/.github/workflows/skills-index-freshness.yml index 4931ccaa0101..9e4b2767be56 100644 --- a/.github/workflows/skills-index-freshness.yml +++ b/.github/workflows/skills-index-freshness.yml @@ -21,6 +21,7 @@ jobs: if: github.repository == 'NousResearch/hermes-agent' runs-on: ubuntu-latest timeout-minutes: 10 + environment: trusted-automation steps: - name: Probe live index id: probe @@ -113,7 +114,7 @@ jobs: id: app-token uses: ./.github/actions/get-app-token with: - client-id: ${{ secrets.APP_CLIENT_ID }} + client-id: ${{ vars.APP_CLIENT_ID }} private-key: ${{ secrets.APP_PRIVATE_KEY }} - name: Open issue on degraded / failed probe diff --git a/.github/workflows/skills-index.yml b/.github/workflows/skills-index.yml index ae05c9e70466..5415499e0245 100644 --- a/.github/workflows/skills-index.yml +++ b/.github/workflows/skills-index.yml @@ -21,6 +21,7 @@ jobs: if: github.repository == 'NousResearch/hermes-agent' runs-on: ubuntu-latest timeout-minutes: 15 + environment: trusted-automation steps: - uses: actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd # v6.0.2 @@ -28,7 +29,7 @@ jobs: id: app-token uses: ./.github/actions/get-app-token with: - client-id: ${{ secrets.APP_CLIENT_ID }} + client-id: ${{ vars.APP_CLIENT_ID }} private-key: ${{ secrets.APP_PRIVATE_KEY }} - uses: actions/setup-python@a309ff8b426b58ec0e2a45f0f869d46889d02405 # v6.2.0 @@ -60,12 +61,13 @@ jobs: if: github.event_name == 'schedule' || github.event_name == 'workflow_dispatch' runs-on: ubuntu-latest timeout-minutes: 15 + environment: trusted-automation steps: - name: Get GitHub App token id: app-token uses: ./.github/actions/get-app-token with: - client-id: ${{ secrets.APP_CLIENT_ID }} + client-id: ${{ vars.APP_CLIENT_ID }} private-key: ${{ secrets.APP_PRIVATE_KEY }} - name: Trigger Deploy Site workflow env: diff --git a/.github/workflows/supply-chain-audit.yml b/.github/workflows/supply-chain-audit.yml index 7ee20e029a7c..cca61e03a452 100644 --- a/.github/workflows/supply-chain-audit.yml +++ b/.github/workflows/supply-chain-audit.yml @@ -65,17 +65,10 @@ jobs: with: fetch-depth: 0 - - name: Get GitHub App token - id: app-token - uses: ./.github/actions/get-app-token - with: - client-id: ${{ secrets.APP_CLIENT_ID }} - private-key: ${{ secrets.APP_PRIVATE_KEY }} - - name: Scan diff for critical patterns id: scan env: - GH_TOKEN: ${{ steps.app-token.outputs.token }} + GH_TOKEN: ${{ github.token }} CI_REVIEWED: ${{ contains(github.event.pull_request.labels.*.name, 'ci-reviewed') }} run: | set -euo pipefail diff --git a/scripts/ci/publish_e2e_evidence.py b/scripts/ci/publish_e2e_evidence.py new file mode 100644 index 000000000000..c381a5990df8 --- /dev/null +++ b/scripts/ci/publish_e2e_evidence.py @@ -0,0 +1,283 @@ +#!/usr/bin/env python3 +"""Publish validated E2E evidence as GitHub attachments and update its PR comment. + +This script only runs from the trusted ``workflow_run`` publisher. It never +checks out PR code: it accepts the small evidence artifact produced by the +untrusted E2E workflow, validates its manifest and PNG bytes, uploads the +approved files as GitHub attachments, and replaces the placeholder in the +source PR's CI review comment with those attachment URLs. +""" + +from __future__ import annotations + +import argparse +import json +import os +import re +import subprocess +import time +import urllib.request +from dataclasses import dataclass +from pathlib import Path +from typing import Any + +API_BASE = "https://api.github.com" +EVIDENCE_START = "" +EVIDENCE_END = "" +PNG_SIGNATURE = b"\x89PNG\r\n\x1a\n" +MAX_FILES = 20 +MAX_FILE_BYTES = 5 * 1024 * 1024 +MAX_TOTAL_BYTES = 20 * 1024 * 1024 +MAX_DIMENSION = 8_000 +COMMENT_LOOKUP_ATTEMPTS = 6 +COMMENT_LOOKUP_DELAY_SECONDS = 2 +_SAFE_FILE = re.compile(r"^[A-Za-z0-9][A-Za-z0-9._-]*\.png$") +_ATTACHMENT_URL = re.compile(r"^!\[[^\]\r\n]*\]\((https://github\.com/user-attachments/assets/[0-9a-fA-F-]+)\)$") + + +@dataclass(frozen=True) +class EvidenceFile: + """One validated PNG and the label used when rendering the PR comment.""" + + filename: str + label: str + + +def _api_request( + url: str, + token: str, + method: str = "GET", + payload: dict[str, Any] | None = None, +) -> dict[str, Any]: + """Send one authenticated GitHub API request and return its JSON object.""" + data = json.dumps(payload).encode("utf-8") if payload is not None else None + request = urllib.request.Request( + url, + data=data, + method=method, + headers={ + "Authorization": f"Bearer {token}", + "Accept": "application/vnd.github+json", + "Content-Type": "application/json", + "X-GitHub-Api-Version": "2022-11-28", + "User-Agent": "hermes-e2e-evidence-publisher", + }, + ) + with urllib.request.urlopen(request) as response: + parsed = json.loads(response.read()) + if not isinstance(parsed, dict): + raise ValueError(f"Expected an object from {url}") + return parsed + + +def _read_png(path: Path) -> bytes: + """Read a bounded PNG, rejecting corrupt and unexpectedly large images.""" + if not path.is_file() or path.is_symlink(): + raise ValueError(f"Evidence file is not a regular file: {path.name}") + size = path.stat().st_size + if size == 0 or size > MAX_FILE_BYTES: + raise ValueError(f"Evidence file has invalid size: {path.name}") + data = path.read_bytes() + if not data.startswith(PNG_SIGNATURE) or len(data) < 24 or data[12:16] != b"IHDR": + raise ValueError(f"Evidence file is not a PNG: {path.name}") + width = int.from_bytes(data[16:20], "big") + height = int.from_bytes(data[20:24], "big") + if not 0 < width <= MAX_DIMENSION or not 0 < height <= MAX_DIMENSION: + raise ValueError(f"Evidence image has invalid dimensions: {path.name}") + return data + + +def _manifest_files(manifest: dict[str, Any]) -> list[EvidenceFile]: + """Flatten a version-one manifest into ordered, reviewer-facing images.""" + if manifest.get("version") != 1: + raise ValueError("Unsupported E2E evidence manifest version") + + files: list[EvidenceFile] = [] + screenshots = manifest.get("screenshots", []) + diffs = manifest.get("diffs", []) + if not isinstance(screenshots, list) or not isinstance(diffs, list): + raise ValueError("Evidence manifest lists are malformed") + + for entry in screenshots: + if not isinstance(entry, dict) or not isinstance(entry.get("name"), str) or not isinstance(entry.get("file"), str): + raise ValueError("Evidence screenshot entry is malformed") + files.append(EvidenceFile(entry["file"], f"new screenshot: {entry['name']}")) + + for entry in diffs: + if not isinstance(entry, dict) or not isinstance(entry.get("name"), str) or not isinstance(entry.get("diff"), str): + raise ValueError("Evidence visual-diff entry is malformed") + files.append(EvidenceFile(entry["diff"], f"visual diff: {entry['name']}")) + for kind in ("actual", "expected"): + value = entry.get(kind) + if value is not None: + if not isinstance(value, str): + raise ValueError("Evidence visual-diff companion is malformed") + files.append(EvidenceFile(value, f"visual {kind}: {entry['name']}")) + + names = [item.filename for item in files] + if len(files) > MAX_FILES or len(set(names)) != len(names): + raise ValueError("Evidence manifest has too many or duplicate files") + if any(not _SAFE_FILE.fullmatch(name) for name in names): + raise ValueError("Evidence manifest contains an unsafe filename") + return files + + +def load_evidence(evidence_dir: Path) -> tuple[list[EvidenceFile], dict[str, bytes]]: + """Load the manifest and return only the validated files it declares.""" + manifest_path = evidence_dir / "e2e-evidence.json" + if not manifest_path.is_file() or manifest_path.is_symlink(): + raise ValueError("E2E evidence manifest is missing") + try: + manifest = json.loads(manifest_path.read_text(encoding="utf-8")) + except json.JSONDecodeError as exc: + raise ValueError("E2E evidence manifest is not JSON") from exc + if not isinstance(manifest, dict): + raise ValueError("E2E evidence manifest is not an object") + + files = _manifest_files(manifest) + payloads: dict[str, bytes] = {} + total = 0 + for item in files: + path = evidence_dir / item.filename + if path.parent != evidence_dir: + raise ValueError("Evidence file escaped its artifact directory") + payload = _read_png(path) + total += len(payload) + if total > MAX_TOTAL_BYTES: + raise ValueError("E2E evidence exceeds the total size limit") + payloads[item.filename] = payload + return files, payloads + + +def render_evidence(files: list[EvidenceFile], attachment_urls: dict[str, str]) -> str: + """Render validated GitHub attachment URLs inside the review-comment marker.""" + blocks = [EVIDENCE_START] + for item in files: + url = attachment_urls.get(item.filename) + if url is None: + raise ValueError(f"Missing attachment URL for {item.filename}") + blocks.extend(( + "
", + f"{item.label}", + "", + f"![{item.label}]({url})", + "", + "
", + )) + blocks.append(EVIDENCE_END) + return "\n".join(blocks) + + +def replace_evidence_marker(comment: str, evidence: str) -> str: + """Replace exactly the pending-evidence region in a CI review comment.""" + pattern = re.compile(f"{re.escape(EVIDENCE_START)}.*?{re.escape(EVIDENCE_END)}", re.DOTALL) + result, count = pattern.subn(evidence, comment, count=1) + if count != 1: + raise ValueError("CI review comment does not contain one evidence marker") + return result + + +def _find_review_comment(comments: object) -> dict[str, Any] | None: + """Find a live CI review comment only after it contains this marker.""" + if not isinstance(comments, list): + raise ValueError("GitHub comments response is malformed") + for item in comments: + if not isinstance(item, dict): + continue + body = str(item.get("body", "")) + if body.startswith("") and EVIDENCE_START in body and EVIDENCE_END in body: + return item + return None + + +def _wait_for_review_comment(token: str, source_repo: str, pr_number: str) -> dict[str, Any]: + """Wait briefly for GitHub's comment API to expose the completed marker.""" + request = urllib.request.Request( + f"{API_BASE}/repos/{source_repo}/issues/{pr_number}/comments?per_page=100", + headers={ + "Authorization": f"Bearer {token}", + "Accept": "application/vnd.github+json", + "X-GitHub-Api-Version": "2022-11-28", + "User-Agent": "hermes-e2e-evidence-publisher", + }, + ) + for attempt in range(COMMENT_LOOKUP_ATTEMPTS): + with urllib.request.urlopen(request) as response: + comment = _find_review_comment(json.loads(response.read())) + if comment is not None: + return comment + if attempt + 1 < COMMENT_LOOKUP_ATTEMPTS: + time.sleep(COMMENT_LOOKUP_DELAY_SECONDS) + raise ValueError("CI review comment with E2E evidence marker is missing") + + +def upload_evidence( + files: list[EvidenceFile], + evidence_dir: Path, + source_repo: str, + session_token: str, +) -> dict[str, str]: + """Upload validated files through gh-image and accept only attachment URLs.""" + environment = os.environ.copy() + environment["GH_SESSION_TOKEN"] = session_token + attachment_urls: dict[str, str] = {} + for item in files: + result = subprocess.run( + ["gh", "image", "--repo", source_repo, str(evidence_dir / item.filename)], + check=True, + capture_output=True, + text=True, + env=environment, + ) + match = _ATTACHMENT_URL.fullmatch(result.stdout.strip()) + if match is None: + raise ValueError(f"gh-image returned an invalid attachment reference for {item.filename}") + attachment_urls[item.filename] = match.group(1) + return attachment_urls + + +def publish( + token: str, + source_repo: str, + evidence_dir: Path, + pr_number: str, + session_token: str, +) -> bool: + """Publish evidence and patch its source PR comment; false means nothing to show.""" + files, _ = load_evidence(evidence_dir) + if not files: + print("No inline E2E evidence to publish.") + return False + comment = _wait_for_review_comment(token, source_repo, pr_number) + attachment_urls = upload_evidence(files, evidence_dir, source_repo, session_token) + evidence = render_evidence(files, attachment_urls) + body = replace_evidence_marker(str(comment.get("body", "")), evidence) + _api_request( + f"{API_BASE}/repos/{source_repo}/issues/comments/{comment['id']}", + token, + method="PATCH", + payload={"body": body}, + ) + print(f"Published {len(files)} E2E evidence image attachment(s).") + return True + + +def main() -> int: + parser = argparse.ArgumentParser(description=__doc__) + parser.add_argument("--evidence-dir", type=Path, required=True) + parser.add_argument("--source-repo", required=True) + parser.add_argument("--pr-number", required=True) + args = parser.parse_args() + + token = os.environ.get("GITHUB_TOKEN", "") + if not token: + parser.error("GITHUB_TOKEN is required") + session_token = os.environ.get("GH_SESSION_TOKEN", "") + if not session_token: + parser.error("GH_SESSION_TOKEN is required") + publish(token, args.source_repo, args.evidence_dir, args.pr_number, session_token) + return 0 + + +if __name__ == "__main__": + raise SystemExit(main()) diff --git a/tests/ci/test_publish_e2e_evidence.py b/tests/ci/test_publish_e2e_evidence.py new file mode 100644 index 000000000000..23e492e762de --- /dev/null +++ b/tests/ci/test_publish_e2e_evidence.py @@ -0,0 +1,136 @@ +"""Tests for scripts/ci/publish_e2e_evidence.py.""" + +from __future__ import annotations + +import importlib.util +import sys +from pathlib import Path + +import pytest + +_PATH = Path(__file__).resolve().parents[2] / "scripts" / "ci" / "publish_e2e_evidence.py" +_spec = importlib.util.spec_from_file_location("publish_e2e_evidence", _PATH) +if _spec is None or _spec.loader is None: + raise ImportError("Failed to load publish_e2e_evidence.py") +_mod = importlib.util.module_from_spec(_spec) +sys.modules["publish_e2e_evidence"] = _mod +_spec.loader.exec_module(_mod) + + +def _png(width: int = 4, height: int = 3) -> bytes: + return _mod.PNG_SIGNATURE + b"\x00\x00\x00\rIHDR" + width.to_bytes(4, "big") + height.to_bytes(4, "big") + + +def test_load_evidence_validates_manifest_and_pngs(tmp_path): + (tmp_path / "shot.png").write_bytes(_png()) + (tmp_path / "diff.png").write_bytes(_png()) + (tmp_path / "actual.png").write_bytes(_png()) + (tmp_path / "expected.png").write_bytes(_png()) + (tmp_path / "e2e-evidence.json").write_text( + """{ + "version": 1, + "screenshots": [{"name": "main-view.png", "file": "shot.png"}], + "diffs": [{"name": "main-view", "diff": "diff.png", "actual": "actual.png", "expected": "expected.png"}] + }""", + encoding="utf-8", + ) + + files, payloads = _mod.load_evidence(tmp_path) + + assert [item.label for item in files] == [ + "new screenshot: main-view.png", + "visual diff: main-view", + "visual actual: main-view", + "visual expected: main-view", + ] + assert set(payloads) == {"shot.png", "diff.png", "actual.png", "expected.png"} + + +def test_load_evidence_rejects_path_escape_and_non_png(tmp_path): + (tmp_path / "e2e-evidence.json").write_text( + '{"version":1,"screenshots":[{"name":"bad","file":"../secret.png"}],"diffs":[]}', + encoding="utf-8", + ) + + with pytest.raises(ValueError, match="unsafe filename"): + _mod.load_evidence(tmp_path) + + (tmp_path / "e2e-evidence.json").write_text( + '{"version":1,"screenshots":[{"name":"bad","file":"not-png.png"}],"diffs":[]}', + encoding="utf-8", + ) + (tmp_path / "not-png.png").write_bytes(b"not a png") + + with pytest.raises(ValueError, match="not a PNG"): + _mod.load_evidence(tmp_path) + + +def test_render_and_replace_evidence_uses_validated_attachment_urls(): + evidence = _mod.render_evidence( + [_mod.EvidenceFile("shot.png", "new screenshot: shot.png")], + {"shot.png": "https://github.com/user-attachments/assets/12345678-1234-1234-1234-123456789abc"}, + ) + body = "before\n\npending\n\nafter" + + result = _mod.replace_evidence_marker(body, evidence) + + assert "pending" not in result + assert "https://github.com/user-attachments/assets/12345678-1234-1234-1234-123456789abc" in result + assert result.startswith("before\n") + assert result.endswith("\nafter") + + +def test_upload_evidence_accepts_only_attachment_urls(tmp_path, monkeypatch): + shot = tmp_path / "shot.png" + shot.write_bytes(_png()) + calls = [] + + def fake_run(args, **kwargs): + calls.append((args, kwargs)) + return _mod.subprocess.CompletedProcess( + args, + 0, + stdout="![shot.png](https://github.com/user-attachments/assets/12345678-1234-1234-1234-123456789abc)\n", + ) + + monkeypatch.setattr(_mod.subprocess, "run", fake_run) + + result = _mod.upload_evidence( + [_mod.EvidenceFile("shot.png", "new screenshot: shot.png")], + tmp_path, + "NousResearch/hermes-agent", + "bot-session-token", + ) + + assert result == {"shot.png": "https://github.com/user-attachments/assets/12345678-1234-1234-1234-123456789abc"} + assert calls[0][0] == ["gh", "image", "--repo", "NousResearch/hermes-agent", str(shot)] + assert calls[0][1]["env"]["GH_SESSION_TOKEN"] == "bot-session-token" + + +def test_upload_evidence_rejects_unexpected_gh_image_output(tmp_path, monkeypatch): + (tmp_path / "shot.png").write_bytes(_png()) + + def fake_run(args, **kwargs): + return _mod.subprocess.CompletedProcess(args, 0, stdout="https://example.invalid/shot.png\n") + + monkeypatch.setattr(_mod.subprocess, "run", fake_run) + + with pytest.raises(ValueError, match="invalid attachment reference"): + _mod.upload_evidence( + [_mod.EvidenceFile("shot.png", "new screenshot: shot.png")], + tmp_path, + "NousResearch/hermes-agent", + "bot-session-token", + ) + + +def test_find_review_comment_requires_the_evidence_marker(): + pending = "\n\npending\n" + + assert _mod._find_review_comment([{"body": " no evidence"}]) is None + assert _mod._find_review_comment([{"body": pending, "id": 123}]) == {"body": pending, "id": 123} + + +def test_replace_evidence_marker_requires_exactly_one_marker(): + with pytest.raises(ValueError, match="does not contain one"): + _mod.replace_evidence_marker("no marker", "evidence")