diff --git a/README.md b/README.md index 824bca5..8208b68 100644 --- a/README.md +++ b/README.md @@ -4,7 +4,7 @@ [![Used by](https://img.shields.io/static/v1?label=Used%20by&message=165&color=informational&logo=slickpic)](https://github.com/commit-check/commit-check-action/network/dependents) [![GitHub marketplace](https://img.shields.io/badge/Marketplace-commit--check--action-blue)](https://github.com/marketplace/actions/commit-check-action) [![commit-check](https://img.shields.io/badge/commit--check-enabled-brightgreen?logo=Git&logoColor=white&color=%232c9ccd)](https://github.com/commit-check/commit-check) -[![slsa-badge](https://slsa.dev/images/gh-badge-level3.svg?color=blue)](https://github.com/commit-check/commit-check-action/blob/a2873ca0482dd505c93fb51861c953e82fd0a186/action.yml#L59-L69) +[![slsa-badge](https://slsa.dev/images/gh-badge-level3.svg?color=blue)](https://github.com/commit-check/commit-check-action/blob/main/action.yml#L84-L94) [![codecov](https://codecov.io/gh/commit-check/commit-check-action/graph/badge.svg?token=QHUDSMJGS7)](https://codecov.io/gh/commit-check/commit-check-action) A GitHub Action for checking commit message formatting, branch naming, committer name, email, commit signoff, and more. @@ -24,11 +24,12 @@ A GitHub Action for checking commit message formatting, branch naming, committer ## Table of Contents * [Usage](#usage) +* [Action, pre-commit hook, or GitHub App — which to use?](#action-pre-commit-hook-or-github-app--which-to-use) * [Optional Inputs](#optional-inputs) * [GitHub Action Job Summary](#github-action-job-summary) * [GitHub Pull Request Comments](#github-pull-request-comments) * [Advanced Configuration](#advanced-configuration) -* [Fork PR Comments](docs/fork-pr-comments.md) +* [Fork Pull Requests](docs/fork-pr-comments.md) * [Badging Your Repository](#badging-your-repository) * [Versioning](#versioning) @@ -52,7 +53,10 @@ jobs: steps: - uses: actions/checkout@v7 with: - fetch-depth: 0 # With a shallow clone only HEAD, the merge commit, is checked + # Required. With the default fetch-depth: 1 the clone holds only GitHub's + # merge commit: the PR's own commits cannot be listed, author checks are + # skipped, and the action warns and falls back to checking HEAD alone. + fetch-depth: 0 - uses: commit-check/commit-check-action@v2 with: message: true @@ -63,9 +67,52 @@ jobs: pr-comments: true ``` +> [!WARNING] +> Without `fetch-depth: 0` the action still runs, but it cannot see the pull +> request's commits. It posts `::warning title=commit-check::Could not list the +> pull request's commits (is actions/checkout using fetch-depth: 0?); only HEAD +> was checked` and checks only the synthetic merge commit — whose subject +> `Merge into ` passes the default rules — so a shallow clone makes +> every PR look green. Author checks are skipped (`⊘`) for the same reason. On +> `pull_request_target`, also check out `refs/pull//merge`. + > [!NOTE] > This action supports running on Linux, macOS, and Windows (`ubuntu-latest`, `macos-latest`, `windows-latest`). +### Runner requirements + +The action is a composite step and uses what the runner already has: + +- **Python 3.10 or newer** on `PATH` (`python3`, or `python` on Windows). No + `setup-python` step is needed on GitHub-hosted runners. Everything the action + installs goes under `$RUNNER_TEMP`, never into your checkout. +- **`gh` CLI** — used to verify the build-provenance attestation of the + `commit-check` wheel before installing it. Present on GitHub-hosted images; + install it on self-hosted runners or the attestation step fails. + Only the `commit-check` wheel is attested; PyGithub and the transitive + dependencies are pinned by `requirements.txt` but not verified. +- **Network access to PyPI and `api.github.com`** — the pinned wheels are + downloaded once per run and the attestation is fetched from GitHub. +- **`git`** on `PATH`, and a checkout with `fetch-depth: 0` (see above). + +There is currently no input to skip attestation verification. + +## Action, pre-commit hook, or GitHub App — which to use? + +All three run the same `commit-check` engine against the same +`commit-check.toml` / `cchk.toml`; they differ in where they run and what they +can see. + +| | GitHub Action (this repo) | [pre-commit hook](https://github.com/commit-check/commit-check#use-with-pre-commit) | [Commit Check GitHub App](https://github.com/marketplace/commit-check) | +|---|---|---|---| +| **Where it runs** | In your workflow, on the runner, after the push | On the contributor's machine, at `git commit` / `git push` | Hosted by commit-check; installed on the repository, no workflow file | +| **What it checks** | Every PR commit's message, plus the PR title, branch and author checks you enable; renders a job summary, annotations, a PR comment and the `result` output | Message (`commit-msg` stage), branch, author; tag, force-push and files (`pre-push`) — one commit at a time, before it exists | Every commit of a push or pull request: message, branch, author (the PR title only in squash mode); reported as one **Commit Check** check run per commit | +| **When to pick it** | You want enforcement in CI that a contributor cannot skip, per-rule outputs for later steps, or you run on GitHub Enterprise Server / need `CCHK_*` overrides | You want the fastest feedback and to stop bad commits before they are pushed; pair it with the Action, since hooks are opt-in | You want zero YAML and no Actions minutes, or feedback on [fork pull requests](docs/fork-pr-comments.md) without the Action's read-only-token limits | + +Most teams pair the pre-commit hook (fast, local) with the Action (enforced): +the hook catches a bad message before it is pushed, and the Action is why CI +fails when a contributor did not install the hook. + ## Used By

@@ -133,9 +180,19 @@ jobs: > [!NOTE] > `pr-comments` is disabled by default. > -> PR comments are skipped for pull requests from forked repositories. See -> [docs/fork-pr-comments.md](docs/fork-pr-comments.md) for details on how to enable -> this feature for fork contributions. +> PR comments are skipped for pull requests from forked repositories, whose +> `GITHUB_TOKEN` is read-only. Everything else still works there: the check +> status, the annotations and the job summary. See +> [Fork pull requests](docs/fork-pr-comments.md). +> +> **Dependabot pull requests** are not forks, but GitHub gives their +> `pull_request` runs a read-only `GITHUB_TOKEN` by default. The `permissions` +> key is honoured for them, so the `pull-requests: write` grant in the +> [usage example](#usage) is enough; without it the action logs a +> `::warning::` on the 403 and leaves the report in the job summary. Note that +> Actions secrets are not available in Dependabot-triggered runs. Adding +> `dependabot[bot]` to `ignore_authors` skips the checks for those PRs +> altogether. > > Note: write-access to pull-requests requires the `pull-requests: write` permission. > See [usage example](#usage). @@ -463,10 +520,13 @@ By default, commit-check-action handles this gracefully: - A **notice is added to the Job Summary** explaining why and how to fix it - The commit checks themselves **still run normally** -> **For most projects, this is sufficient** — contributors can see check results in the -> action Job Summary. But if you *must* have PR comments on fork contributions, see -> the **[Fork PR Comments](docs/fork-pr-comments.md)** documentation for -> two recommended approaches with ready-to-use workflow examples. +> **For most projects, this is sufficient** — a fork contributor already gets the red +> check, the per-finding annotations on their diff and the full report in the job +> summary. If you want feedback on the pull request itself, the +> [Commit Check GitHub App](https://github.com/marketplace/commit-check) posts a check +> run per commit with no workflow file (free on public repositories), or you can run +> this action on `pull_request_target`. Both are covered in +> **[Fork pull requests](docs/fork-pr-comments.md)**. ## Badging Your Repository diff --git a/action.yml b/action.yml index f0566de..153cb01 100644 --- a/action.yml +++ b/action.yml @@ -55,31 +55,43 @@ runs: # Platform-specific settings if [[ "$RUNNER_OS" == "Windows" ]]; then PYTHON_CMD="python" - VENV_ACTIVATE="venv/Scripts/activate" else if [[ "$RUNNER_OS" == "Linux" ]]; then # https://github.com/pypa/setuptools/issues/3269 export DEB_PYTHON_INSTALL_LAYOUT=deb fi PYTHON_CMD="python3" - VENV_ACTIVATE="venv/bin/activate" fi - # Set up virtual environment - $PYTHON_CMD -m venv venv - source "$VENV_ACTIVATE" + # Everything the action installs lives under $RUNNER_TEMP, never in the + # caller's checkout: a later `git status`, linter or upload-artifact step + # must not see our venv or wheels. + WORK="$RUNNER_TEMP/commit-check-action" + rm -rf "$WORK" # a second invocation in the same job starts clean, so the wheel glob below matches exactly one file + mkdir -p "$WORK/wheels" + $PYTHON_CMD -m venv "$WORK/venv" + if [[ "$RUNNER_OS" == "Windows" ]]; then + source "$WORK/venv/Scripts/activate" + else + source "$WORK/venv/bin/activate" + fi - # Download artifact - $PYTHON_CMD -m pip download -r "$GITHUB_ACTION_PATH/requirements.txt" + # One download of the pinned closure, into the scratch dir. Wheels only: + # the offline install below cannot build an sdist, so a platform without + # binary wheels fails here, with pip's message, rather than later. + $PYTHON_CMD -m pip download -q --only-binary=:all: -d "$WORK/wheels" -r "$GITHUB_ACTION_PATH/requirements.txt" - # Verify artifact attestations - if ! gh attestation verify commit_check-*.whl -R commit-check/commit-check; then - echo "Artifact verification failed. Aborting installation." - exit 1 + # Verify the commit-check wheel's build provenance (PyGithub and the + # transitive wheels are pinned but not attested; see README "Runner requirements"). + WHEEL=$(ls "$WORK"/wheels/commit_check-*.whl) # exactly one: pip download of a == pin + if ! gh attestation verify "$WHEEL" -R commit-check/commit-check; then + echo "::error::Attestation verification failed for $(basename "$WHEEL") (self-hosted runners need the gh CLI). Aborting installation." + exit 1 fi - # Install artifact - $PYTHON_CMD -m pip install commit_check-*.whl pygithub-*.whl + # Install offline from the wheels we already downloaded and verified: + # no second trip to PyPI, and the installed set is exactly the downloaded set. + $PYTHON_CMD -m pip install -q --no-cache-dir --no-index --find-links "$WORK/wheels" -r "$GITHUB_ACTION_PATH/requirements.txt" $PYTHON_CMD "$GITHUB_ACTION_PATH/main.py" env: diff --git a/docs/fork-pr-comments.md b/docs/fork-pr-comments.md index b267011..b288173 100644 --- a/docs/fork-pr-comments.md +++ b/docs/fork-pr-comments.md @@ -1,155 +1,77 @@ -# Fork PR Comments +# Fork Pull Requests -When a pull request is opened from a **forked repository**, the `GITHUB_TOKEN` used by the -`pull_request` event has **read-only** permissions by design (GitHub security policy). -This means `pr-comments: true` cannot write a comment back to the PR. +When a pull request comes from a **forked repository**, the `GITHUB_TOKEN` of the +`pull_request` event is **read-only** by design (GitHub security policy). That single +restriction is worth understanding precisely, because it costs less than it sounds like. -By default, commit-check-action handles this gracefully: +## What a fork contributor still sees -- PR comment writing is **skipped** with a `::warning::` message in the logs -- A **notice is added to the Job Summary** explaining why and how to fix it -- The commit checks themselves **still run normally** +Everything except the comment. The action does not degrade on a fork PR: -> **For most projects, this is sufficient** — contributors can see check results in the -> action Job Summary. But if you *must* have PR comments on fork contributions, there -> are two recommended approaches. +| Surface | Fork PR | Why | +|---|---|---| +| The check's pass/fail status | ✅ works | the job's own conclusion | +| `::error` annotations on the **Files changed** tab | ✅ works | workflow commands are written by the runner, not the API | +| The **job summary** — the full report table and details | ✅ works | `$GITHUB_STEP_SUMMARY` is a file on the runner | +| The `result` output for later steps | ✅ works | `$GITHUB_OUTPUT` is a file on the runner | +| A **PR comment** | ❌ skipped | writing a comment needs the API, and the token is read-only | ---- - -## Option 1: Two-workflow pattern (recommended) - -This is the **official GitHub-recommended best practice** for writing PR comments from -fork PRs. It uses the [`workflow_run`](https://docs.github.com/en/actions/using-workflows/events-that-trigger-workflows#workflow_run) -event with **no security risks**. - -> 📁 Ready-to-use files: [`examples/commit-check-workflow-a.yml`](../examples/commit-check-workflow-a.yml) -> and [`examples/commit-check-workflow-b.yml`](../examples/commit-check-workflow-b.yml) - -**How it works:** +So a contributor pushing to a fork already gets the red check, the per-finding annotations +on their diff, and the whole report in the job summary. The action says so in the log: ``` - pull_request workflow_run - │ │ - ▼ ▼ -┌──────────────┐ ┌──────────────────┐ -│ Workflow A │ │ Workflow B │ -│ (checks) │────►│ (comment writer) │ -│ │ │ │ -│ Token: READ │ │ Token: WRITE │ -│ Saves result │ │ Reads artifact │ -│ as artifact │ │ Posts PR comment │ -└──────────────┘ └──────────────────┘ +::warning::Skipping PR comment: pull requests from forked repositories cannot write +comments via the pull_request event (GITHUB_TOKEN is read-only for forks). The findings +are in this job's summary and in the annotations on the Files changed tab. ``` -### Workflow A +The run is **not** failed by this: `pr-comments: true` on a fork PR is a no-op, not an error. -`.github/workflows/commit-check.yml` (triggered by `pull_request`): +## If you want feedback on the pull request itself -```yaml -name: Commit Check +### Install the Commit Check GitHub App (recommended) -on: - pull_request: - branches: ["main"] +The [Commit Check GitHub App](https://github.com/marketplace/commit-check) is not bound by +the `pull_request` token at all: it receives the `pull_request` webhook on the **base** +repository and acts with its own installation token. Fork pull requests are ordinary +pull requests to it. -jobs: - check: - runs-on: ubuntu-latest - steps: - - uses: actions/checkout@v7 - with: - fetch-depth: 0 - - uses: commit-check/commit-check-action@v2 - with: - message: true - branch: true - pr-comments: false # comments handled by Workflow B - job-summary: true - - uses: actions/upload-artifact@v4 - with: - name: commit-check-result-${{ github.event.number }} - path: result.txt # saved for Workflow B -``` +- **No workflow file.** Install it on the repository and it runs. +- **One check run per commit**, with the failing value, the rule and the suggested fix. + A check run is a first-class PR surface: it shows in the merge box and links straight + to the details. +- **Free on public repositories and personal accounts** — which is where fork pull + requests happen. Private organization repositories need the Team plan. -> 📄 Full file: [`examples/commit-check-workflow-a.yml`](../examples/commit-check-workflow-a.yml) +It reports as a check run, not as a comment. If your goal is "the contributor sees what +failed, on the pull request, without me writing YAML", this is the shortest path. -### Workflow B +You can run the App and this action together: the App covers fork pull requests, the +action gives you enforcement you control in CI, per-rule outputs for later steps, and +`CCHK_*` overrides. -`.github/workflows/commit-check-comment.yml` (triggered by `workflow_run`): +### Or run on `pull_request_target` -```yaml -name: Commit Check Comment - -on: - workflow_run: - workflows: ["Commit Check"] # must match Workflow A's name exactly - types: [completed] - -jobs: - comment: - runs-on: ubuntu-latest - permissions: - pull-requests: write - actions: read # needed to download artifacts - steps: - - uses: actions/download-artifact@v4 - with: - name: commit-check-result-${{ github.event.workflow_run.pull_requests[0].number }} - run-id: ${{ github.event.workflow_run.id }} - github-token: ${{ github.token }} - - name: Read result and post PR comment - uses: actions/github-script@v7 - with: - script: | - // See examples/commit-check-workflow-b.yml for full script - const fs = require('fs'); - const prNumber = ${{ github.event.workflow_run.pull_requests[0].number }}; - const resultText = fs.readFileSync('result.txt', 'utf8').trim(); - const body = resultText - ? '# Commit-Check ❌\n```\n' + resultText + '\n```' - : '# Commit-Check ✔️'; - // Creates or updates the matching PR comment -``` - -> 📄 Full file: [`examples/commit-check-workflow-b.yml`](../examples/commit-check-workflow-b.yml) - -### Key security benefits - -- Workflow B runs in the **base repository's context**, so `GITHUB_TOKEN` has full write - permissions (you explicitly grant `pull-requests: write`) -- Workflow B **does not checkout the PR code**, so untrusted fork code never runs - with elevated permissions -- The artifact only contains `result.txt` — no code or secrets - ---- - -## Option 2: pull_request_target (advanced, use with caution) - -If you understand the security implications, you can use -[`pull_request_target`](https://docs.github.com/en/actions/using-workflows/events-that-trigger-workflows#pull_request_target) -which runs in the base repository's context with **write token access**. - -> **⚠️ Security warning:** Never check out (`actions/checkout`) the PR's HEAD commit -> when using `pull_request_target`. Always check out the base branch or use the -> default merge commit. Otherwise, fork code could exfiltrate your repository's secrets. +If you cannot install a GitHub App — GitHub Enterprise Server, or an organization policy +that forbids it — `pull_request_target` runs in the context of the base repository, so +`GITHUB_TOKEN` has the permissions your workflow asks for and `pr-comments: true` works +on fork pull requests. ```yaml -name: Commit Check - on: pull_request_target: - branches: ["main"] + +permissions: + contents: read + pull-requests: write jobs: commit-check: runs-on: ubuntu-latest - permissions: - contents: read - pull-requests: write steps: - # SAFE: checkout the merge commit, NOT the PR head - uses: actions/checkout@v7 with: + ref: refs/pull/${{ github.event.number }}/merge # the PR's commits fetch-depth: 0 - uses: commit-check/commit-check-action@v2 with: @@ -158,9 +80,18 @@ jobs: pr-comments: true ``` -> ✅ With `pull_request_target`, `pr-comments: true` **does work** on fork PRs — -> the token has the workflow's configured permissions regardless of whether the PR -> is from a fork. -> -> **When to use this:** Only if the two-workflow pattern is too complex for your setup -> and you have thoroughly reviewed the security implications. +> [!WARNING] +> `pull_request_target` grants a writable token to a workflow whose checkout contains the +> fork's code. commit-check only *reads* commit metadata and never executes the checked-out +> tree, but any other step you add to this job runs with that token. Keep the job to the +> checkout and this action, never cache or build from it, and never expose secrets to it. +> See [GitHub's guidance on `pull_request_target`](https://securitylab.github.com/resources/github-actions-preventing-pwn-requests/). + +## What this page used to describe + +Earlier versions documented a two-workflow pattern: workflow A runs the checks on +`pull_request` and uploads an artifact, workflow B picks it up on `workflow_run` and posts +the comment with a writable token. It worked, but it cost two workflow files, an artifact +round trip, `actions: read`, and smuggling the PR number through the artifact — all to move +information the contributor could already see into a comment. The App does the same job with +no YAML at all, so the pattern and its example workflows have been removed. diff --git a/examples/commit-check-workflow-a.yml b/examples/commit-check-workflow-a.yml deleted file mode 100644 index 8c6c2ba..0000000 --- a/examples/commit-check-workflow-a.yml +++ /dev/null @@ -1,33 +0,0 @@ -# Workflow A: Run commit checks on pull_request events. -# -# This workflow is triggered by pull_request and runs commit checks. -# It uploads the result as an artifact so Workflow B (commit-check-comment.yml) -# can read it and post a PR comment with full write permissions. -# -# See https://github.com/commit-check/commit-check-action#fork-pr-comments - -name: Commit Check - -on: - pull_request: - branches: ["main"] - -jobs: - check: - runs-on: ubuntu-latest - steps: - - uses: actions/checkout@v7 - with: - fetch-depth: 0 - - uses: commit-check/commit-check-action@v2 - with: - message: true - branch: true - pr-comments: false # comments handled by Workflow B - job-summary: true - - # Save results so Workflow B can post a PR comment - - uses: actions/upload-artifact@v4 - with: - name: commit-check-result-${{ github.event.number }} - path: result.txt diff --git a/examples/commit-check-workflow-b.yml b/examples/commit-check-workflow-b.yml deleted file mode 100644 index 09517b8..0000000 --- a/examples/commit-check-workflow-b.yml +++ /dev/null @@ -1,68 +0,0 @@ -# Workflow B: Post PR comment after commit checks complete. -# -# This workflow is triggered by the workflow_run event from Workflow A. -# It runs in the base repository's context with full write permissions, -# making it safe for fork PRs (no checkout of fork code). -# -# Prerequisites: -# - Workflow A (commit-check.yml) must exist and upload an artifact named -# commit-check-result- containing result.txt -# -# See https://github.com/commit-check/commit-check-action#fork-pr-comments - -name: Commit Check Comment - -on: - workflow_run: - workflows: ["Commit Check"] # must match Workflow A's name exactly - types: [completed] - -jobs: - comment: - runs-on: ubuntu-latest - permissions: - pull-requests: write - actions: read # needed to download artifacts - steps: - - uses: actions/download-artifact@v4 - with: - name: commit-check-result-${{ github.event.workflow_run.pull_requests[0].number }} - run-id: ${{ github.event.workflow_run.id }} - github-token: ${{ github.token }} - - - name: Read result and post PR comment - uses: actions/github-script@v7 - with: - script: | - const fs = require('fs'); - const prNumber = ${{ github.event.workflow_run.pull_requests[0].number }}; - const resultText = fs.readFileSync('result.txt', 'utf8').trim(); - - const successTitle = '# Commit-Check ✔️'; - const failureTitle = '# Commit-Check ❌'; - const body = resultText - ? `${failureTitle}\n\`\`\`\n${resultText}\n\`\`\`` - : successTitle; - - const { data: comments } = await github.rest.issues.listComments({ - ...context.repo, - issue_number: prNumber, - }); - - const existing = comments.find(c => - c.body.startsWith(successTitle) || c.body.startsWith(failureTitle) - ); - - if (existing) { - await github.rest.issues.updateComment({ - ...context.repo, - comment_id: existing.id, - body, - }); - } else { - await github.rest.issues.createComment({ - ...context.repo, - issue_number: prNumber, - body, - }); - } diff --git a/main.py b/main.py index 32b9209..23dd4b0 100755 --- a/main.py +++ b/main.py @@ -8,12 +8,16 @@ * **step log** — grouped sections, then one ``::error`` annotation per finding * **job summary** — a Markdown policy report table * **PR comment** — a compact Markdown summary (idempotently updated) + +and exposes the check data as JSON in the ``result`` action output. """ import json import os +import re import subprocess import sys +import uuid from dataclasses import dataclass, field from typing import Any @@ -686,17 +690,23 @@ def _finding_lines(check: dict[str, str], include_error: bool) -> list[str]: so printing both would say the same thing twice in a row; in exactly that case only ``Fix:`` is shown. A multi-line fix (a signed-off body) takes one row per line so the trailer lands where it would in the - message. + message; a multi-line value (a whole commit message on a failing rule) + or suggestion is split the same way, so every line of user text sits + inside the tree rather than at column 0. """ lines: list[str] = [] if check.get("value"): - lines.append(f"value: {check['value']}") + first, *rest = str(check["value"]).splitlines() + lines.append(f"value: {first}") + lines.extend(rest) if include_error: lines.extend(check.get("error", "").splitlines()) fix = check.get("fix", "") suggest = check.get("suggest", "") if suggest and suggest != f'Use "{fix}"': - lines.append(f"Suggest: {suggest}") + first, *rest = suggest.splitlines() + lines.append(f"Suggest: {first}") + lines.extend(rest) if fix: first, *rest = fix.splitlines() lines.append(f"Fix: {first}") @@ -966,7 +976,7 @@ def _markdown_table( if not entries and not raw_failure: continue value = _scope_value(scope) - value_display = f"`{value}`" if value else "\u2014" + value_display = _markdown_code(value) if value else "\u2014" if raw_failure: links = "_output could not be parsed \u2014 see details_" else: @@ -1013,12 +1023,40 @@ def _markdown_details(results: list[ScopeResult]) -> str: _failed, total = _check_counts(results) unit = "check" if total == 1 else "checks" label = f"Show all {total} {unit}" if total else "Show details" - lines = ["

", f"{label}", "", "```text"] - lines.extend(_render_tree(results, include_docs=False)) - lines.extend(["```", "", "
"]) + body = _render_tree(results, include_docs=False) + # The tree quotes commit subjects, errors and suggestions as they are. A + # value with three backticks in it (a `docs:` commit showing a fence) + # would close a fixed ``` fence and spill the rest of the report out as + # prose, so the fence is one longer than any backtick run inside. + fence = "`" * max(3, _longest_backtick_run(body) + 1) + lines = ["
", f"{label}", "", f"{fence}text"] + lines.extend(body) + lines.extend([fence, "", "
"]) return "\n".join(lines) +def _longest_backtick_run(lines: list[str]) -> int: + """Length of the longest run of consecutive backticks across ``lines``.""" + return max((len(m) for line in lines for m in re.findall(r"`+", line)), default=0) + + +def _markdown_code(value: str) -> str: + """A code span that survives a GFM table cell. + + The value is user text (a commit subject, a branch name, an author) and + commonly quotes code. A bare ```` `{value}` ```` breaks twice on that: a + backtick inside closes the span early, and an unescaped ``|`` splits the + cell, pushing the rule link into a fourth column the table drops. So the + span uses one more backtick than the longest run inside the value, pads + with a space when the value starts or ends with a backtick (GFM strips + one on each side, so the padding is invisible), and escapes ``|``, which + GFM honours even inside a code span when the span sits in a table cell. + """ + fence = "`" * (_longest_backtick_run([value]) + 1) + pad = " " if value.startswith("`") or value.endswith("`") else "" + return f"{fence}{pad}{value.replace('|', chr(92) + '|')}{pad}{fence}" + + def _scope_value(scope: ScopeResult, max_len: int = 60) -> str: """First non-empty check value for a scope, trimmed to a single line. @@ -1193,6 +1231,11 @@ def _scope_value(scope: ScopeResult, max_len: int = 60) -> str: # - Values are capped at 60 characters with a literal "..." suffix, except on a # failing scope, where the details block prints the value in full — it is the # one value the reader has to act on and the cap can hide the reason. +# - The Checked value cell is a code span whose backtick fence is longer than +# any backtick run in the value, with `|` escaped, so a subject that quotes +# code cannot close the span or split the row; the details fence likewise +# grows past any backtick run in the tree. Reading the raw Markdown, expect +# ``fix: handle `None` \| retry`` rather than `fix: handle `None` | retry`. # - The step log renders the same tree (_render_scopes); it adds the docs URL, # which the Markdown report already carries on the rule ID in the table. # --------------------------------------------------------------------------- @@ -1297,7 +1340,9 @@ def add_job_summary(results: list[ScopeResult]) -> int: def set_result_output(results: list[ScopeResult]) -> None: """Expose the structured results as the ``result`` action output. - Uses the heredoc form of ``GITHUB_OUTPUT`` so multi-line JSON survives. + Written through :func:`_write_output`, whose per-write random delimiter + keeps a line of user text (a commit subject reading ``EOF``, say) from + closing the heredoc early. """ output_path = os.getenv("GITHUB_OUTPUT") if not output_path: @@ -1315,9 +1360,23 @@ def set_result_output(results: list[ScopeResult]) -> None: ], } with open(output_path, "a", encoding="utf-8") as f: - f.write("result< None: + """Append one multi-line output in the heredoc form ``GITHUB_OUTPUT`` takes. + + The runner reads lines up to the first one equal to the delimiter and + rejects the whole file if it never finds one, which fails the step and + drops every output written after the bad one. A fixed ``EOF`` delimiter + is therefore unsafe: the JSON quotes commit subjects and error text as + they are, and a subject reading ``EOF`` is legal. So the delimiter is + random per write, the shape actions/github-script uses. + """ + delimiter = f"ghadelimiter_{uuid.uuid4()}" + while delimiter in value: # pragma: no cover - 122 random bits + delimiter = f"ghadelimiter_{uuid.uuid4()}" + f.write(f"{name}<<{delimiter}\n{value}\n{delimiter}\n") def is_fork_pr() -> bool: @@ -1421,9 +1480,9 @@ def add_pr_comments(results: list[ScopeResult]) -> int: msg = ( "Skipping PR comment: pull requests from forked repositories " "cannot write comments via the pull_request event (GITHUB_TOKEN is " - "read-only for forks). " - "See https://github.com/commit-check/commit-check-action/blob/main/docs/fork-pr-comments.md " - "for how to enable PR comments on fork PRs." + "read-only for forks). The findings are in this job's summary and " + "in the annotations on the Files changed tab. " + "See https://github.com/commit-check/commit-check-action/blob/main/docs/fork-pr-comments.md" ) print(f"::warning::{msg}") if JOB_SUMMARY_ENABLED and GITHUB_STEP_SUMMARY: @@ -1433,9 +1492,10 @@ def add_pr_comments(results: list[ScopeResult]) -> int: "### \u2139\ufe0f PR Comment Skipped\n\n" "Pull requests from forked repositories cannot write comments " "using the `pull_request` event because `GITHUB_TOKEN` has " - "read-only permissions.\n\n" - "> **\U0001f4a1 Tip:** To enable PR comments on fork PRs, see " - "[Enabling PR Comments on Fork Pull Requests]" + "read-only permissions. The report above and the annotations " + "on the Files changed tab are unaffected.\n\n" + "> **\U0001f4a1 Tip:** see " + "[Fork pull requests]" "(https://github.com/commit-check/commit-check-action/blob/main/docs/fork-pr-comments.md).\n" ) return 0 diff --git a/main_test.py b/main_test.py index e6a585a..a5a106e 100644 --- a/main_test.py +++ b/main_test.py @@ -5,6 +5,7 @@ import json import os import re +import shutil import sys import tempfile import unittest @@ -1727,6 +1728,47 @@ def test_failure_returns_nonzero(self): self.assertIn("❌", content) +def read_github_output(path: str) -> dict[str, str]: + """Parse a ``GITHUB_OUTPUT`` file the way the runner does. + + A port of the loop in actions/runner's ``FileCommandManager``: a line is + either ``name=value`` or ``name<= 0 and (heredoc < 0 or equals < heredoc): + name, value = line.split("=", 1) + outputs[name] = value + continue + if heredoc < 0: + raise ValueError(f"Invalid format '{line}'") + name, delimiter = line.split("<<", 1) + if not delimiter: + raise ValueError("Invalid format: empty delimiter") + body: list[str] = [] + while True: + if index >= len(lines): + raise ValueError(f"Matching delimiter not found '{delimiter}'") + current = lines[index] + index += 1 + if current == delimiter: + break + body.append(current) + outputs[name] = "\n".join(body) + return outputs + + class TestSetResultOutput(unittest.TestCase): def test_writes_heredoc_json(self): output_path = os.path.join(tempfile.mkdtemp(), "output.txt") @@ -1734,10 +1776,44 @@ def test_writes_heredoc_json(self): main.set_result_output([fail_scope("Commit 1/1"), pass_scope("Branch")]) with open(output_path, encoding="utf-8") as file_obj: content = file_obj.read() - self.assertIn("result< main.ScopeResult: + return main.ScopeResult( + label="Commit 1/1", + sha=SHA_B, + checks=[ + make_check( + "message", + status="fail", + rule_id="CC001", + value=value, + error="The commit message should follow Conventional Commits.", + docs_url="https://commit-check.com/rules/#cc001", + ), + make_check("subject_imperative", rule_id="CC003", value=value), + ], + ) + + def test_markdown_code_wraps_backticks_and_escapes_pipes(self): + cases = { + "plain": "`plain`", + "a|b": "`a\\|b`", + "fix: handle `None` | retry": "``fix: handle `None` \\| retry``", + "`x`": "`` `x` ``", + "``x``": "``` ``x`` ```", + "x`": "`` x` ``", + } + for value, expected in cases.items(): + with self.subTest(value=value): + self.assertEqual(main._markdown_code(value), expected) + + def test_table_row_with_backticks_and_a_pipe_keeps_three_cells(self): + table = main._markdown_table([self._scope("fix: handle `None` | retry")]) + row = table.splitlines()[2] + # Strip the outer pipes before counting: "| a | b | c |" -> 3 cells. + cells = self.CELL_SEPARATOR.split(row.strip().strip("|")) + self.assertEqual(len(cells), 3, row) + self.assertEqual(cells[1].strip(), "``fix: handle `None` \\| retry``") + self.assertEqual( + cells[2].strip(), "[CC001 message](https://commit-check.com/rules/#cc001)" + ) + + def test_truncated_value_with_an_unbalanced_backtick_still_closes(self): + """The 60-char cap can cut inside a backtick run; the fence is chosen + after truncation, so the span still closes at the right place.""" + value = "fix: " + "`x`, " * 20 + truncated = main._scope_value(self._scope(value)) + self.assertTrue(truncated.endswith("`x..."), truncated) # cut mid-span + cell = main._markdown_code(truncated) + self.assertEqual(cell, f"``{truncated}``") + + def test_details_fence_outgrows_a_triple_backtick_in_the_value(self): + details = main._markdown_details([self._scope("docs: show ``` usage")]) + lines = details.splitlines() + self.assertEqual(lines[3], "````text") + self.assertEqual(lines[-3], "````") + self.assertIn("value: docs: show ``` usage", details) + + def test_details_fence_stays_three_backticks_for_ordinary_values(self): + details = main._markdown_details([pass_scope(value="feat: add `login`")]) + lines = details.splitlines() + self.assertEqual(lines[3], "```text") + self.assertEqual(lines[-3], "```") + + class TestSkipRenderingEdgeCases(unittest.TestCase): def test_failure_table_omits_skipped_scopes(self): """A skipped scope has no failed checks, so it must not get a row. @@ -2586,3 +2736,77 @@ def test_step_log_partial_skip_does_not_claim_all_passed(self): out = buf.getvalue() self.assertIn("1 of 2 checks passed, 1 skipped", out) self.assertNotIn("all checks passed", out) + + +# --------------------------------------------------------------------------- +# Integration: the real commit-check binary +# --------------------------------------------------------------------------- + +#: The JSON shape ``make_check()`` hard-codes and every renderer reads. +CHECK_KEYS = { + "rule_id", + "check", + "status", + "value", + "error", + "suggest", + "fix", + "docs_url", +} +STATUSES = {"pass", "fail", "warn", "skip"} + + +@unittest.skipUnless( + shutil.which("commit-check"), + "commit-check CLI not on PATH (CI installs it from requirements.txt)", +) +class TestRealCommitCheckBinary(unittest.TestCase): + """Run the pinned commit-check once, unmocked. + + Every other test patches ``main.subprocess.run`` and feeds back the JSON + shape ``make_check()`` hard-codes, so a renamed key or a new status in + the CLI would leave the whole suite green while the action rendered "—" + for every value. This is the one place that drift can fail a build. CI + installs requirements.txt, so the binary is always present there; the + skip only spares a contributor running the suite without it. + """ + + def _run(self, message: str) -> tuple[int, dict]: + rc, data, raw = main.run_check_json(["--message"], input_text=message) + self.assertIsInstance(data, dict, f"CLI did not emit JSON:\n{raw}") + assert data is not None # for the type checker; asserted above + self.assertIn("checks", data) + self.assertTrue(data["checks"], "CLI reported no checks") + for check in data["checks"]: + self.assertEqual(set(check), CHECK_KEYS, check) + self.assertIn(check["status"], STATUSES, check) + self.assertRegex(check["rule_id"], r"^CC\d{3}$") + self.assertTrue( + check["docs_url"].startswith("https://commit-check.com/rules/#") + ) + return rc, data + + def test_passing_message(self): + rc, data = self._run("fix: handle the empty case\n") + self.assertEqual(rc, 0) + self.assertEqual(data["status"], "pass") + self.assertTrue(all(c["status"] == "pass" for c in data["checks"])) + scope = main.ScopeResult(label="Commit 1/1", checks=data["checks"]) + self.assertEqual(scope.status, "pass") + self.assertEqual(main.overall_status([scope]), "pass") + + def test_failing_message(self): + rc, data = self._run("Bad subject\n") + self.assertEqual(rc, 1) + self.assertEqual(data["status"], "fail") + failed = [c for c in data["checks"] if c["status"] == "fail"] + self.assertEqual([c["rule_id"] for c in failed], ["CC001"]) + self.assertTrue(failed[0]["error"]) + scope = main.ScopeResult(label="Commit 1/1", checks=data["checks"]) + self.assertEqual(scope.status, "fail") + self.assertEqual(main.overall_status([scope]), "fail") + # The rendered report must carry the rule link the CLI supplied. + self.assertIn( + "[CC001 message](https://commit-check.com/rules/#cc001)", + main.render_report([scope]), + )