From 39edd80490803fee6f89c5574900ce7279c34200 Mon Sep 17 00:00:00 2001 From: huangruiteng <14976749+huangruiteng@users.noreply.github.com> Date: Sun, 27 Sep 2026 02:39:17 +0800 Subject: [PATCH] fix(ci): exempt verified GitHub integration merges from DCO Signed-off-by: huangruiteng <14976749+huangruiteng@users.noreply.github.com> --- .github/workflows/dco.yml | 35 ++++++++++-- CONTRIBUTING.md | 7 ++- tests/test_dco_workflow.py | 109 +++++++++++++++++++++++++++++++++++++ 3 files changed, 146 insertions(+), 5 deletions(-) diff --git a/.github/workflows/dco.yml b/.github/workflows/dco.yml index 40a5245346..363be25385 100644 --- a/.github/workflows/dco.yml +++ b/.github/workflows/dco.yml @@ -17,10 +17,11 @@ jobs: with: fetch-depth: 0 - - name: Require a DCO trailer on every commit + - name: Require DCO trailers on contribution commits env: BASE_REF: ${{ github.event.pull_request.base.ref }} HEAD_SHA: ${{ github.event.pull_request.head.sha }} + GH_TOKEN: ${{ github.token }} shell: bash run: | set -euo pipefail @@ -36,11 +37,37 @@ jobs: missing=0 while IFS= read -r commit; do [[ -z "${commit}" ]] && continue - if ! git show -s --format=%B "${commit}" | + if git show -s --format=%B "${commit}" | grep -Eq '^Signed-off-by: [^<]+ <[^<>[:space:]]+@[^<>[:space:]]+>$'; then - echo "::error::Commit ${commit} is missing a valid Signed-off-by trailer." - missing=1 + continue fi + + # GitHub adds signed integration commits without DCO trailers. + # A name/email alone is forgeable: require GitHub's verified record + # for this exact two-parent merge. Its parent commits stay in the + # range and are checked independently; manual merges are not exempt. + read -ra parents <<< "$(git show -s --format=%P "${commit}")" + if [[ "${#parents[@]}" -eq 2 ]] && + [[ "$(git show -s --format=%cn "${commit}")" == "GitHub" ]] && + [[ "$(git show -s --format=%ce "${commit}")" == "noreply@github.com" ]]; then + if ! metadata=$(gh api "repos/${GITHUB_REPOSITORY}/commits/${commit}"); then + echo "::error::Cannot verify GitHub merge provenance for ${commit}; retry the check when the API is available." + exit 1 + fi + if jq -e --arg sha "${commit}" --arg first "${parents[0]}" --arg second "${parents[1]}" ' + .sha == $sha and .committer.login == "web-flow" and + .commit.committer.name == "GitHub" and + .commit.committer.email == "noreply@github.com" and + .commit.verification.verified == true and + .commit.verification.reason == "valid" and + ([.parents[].sha] == [$first, $second]) + ' <<< "${metadata}" >/dev/null; then + echo "Verified GitHub-generated merge ${commit}; checking its contributions separately." + continue + fi + fi + echo "::error::Commit ${commit} is missing a valid Signed-off-by trailer." + missing=1 done <<< "${commits}" if [[ "${missing}" -ne 0 ]]; then diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 84d21f536c..ba7bc4c375 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -161,7 +161,7 @@ touch, mention it in the PR instead of fixing it there. | Workflow file | Runs on a PR | Blocks merge | What it checks | | --- | --- | --- | --- | | `python-tests.yml` | every PR | yes (`merge-gate`) | lint, mypy, sharded pytest, TypeScript core and coverage, minimum Node.js, Windows PowerShell, dashboard presentation | -| `dco.yml` | every PR | yes (`Sign-off`) | `Signed-off-by` trailer on every commit | +| `dco.yml` | every PR | yes (`Sign-off`) | `Signed-off-by` on contribution commits; verified GitHub-generated two-parent merges are exempt | | `dependency-review.yml` | every PR | no | dependency changes introduced by the PR | | `postgresql-integration.yml` | control-plane or npm lockfile paths | no | PostgreSQL authority store and service on a temporary instance | | `package-smoke.yml` | extension package paths | no | extension packages install, entrypoints, and example schemas | @@ -211,6 +211,11 @@ be information you are permitted to publish in the permanent Git history. If a commit is missing the trailer, amend it with `git commit --amend -s` or use an interactive rebase to sign the affected commits, then update the pull-request branch. The `DCO` pull-request check rejects unsigned commits. +This includes manual merge commits and web edits. The check exempts only +two-parent integration commits whose exact SHA, parents and `web-flow` identity +have a valid signature verification in GitHub's commit record; it still checks +the underlying contribution commits. A GitHub-looking name or email is not +enough. If the provenance API is unavailable, the check fails with retry guidance. Releases through `v0.4.7` remain under their original MIT terms. See the [licensing and v0.4.8 transition policy](docs/project/licensing.md) for the diff --git a/tests/test_dco_workflow.py b/tests/test_dco_workflow.py index 7450df8d44..7b0ed898f4 100644 --- a/tests/test_dco_workflow.py +++ b/tests/test_dco_workflow.py @@ -3,7 +3,9 @@ from __future__ import annotations import os +import json import subprocess +import sys from pathlib import Path import pytest @@ -74,6 +76,7 @@ def check_dco( "${{ github.event.pull_request.base.sha }}": old_base, "${{ github.event.pull_request.base.ref }}": base_ref, "${{ github.event.pull_request.head.sha }}": head, + "${{ github.token }}": "synthetic-read-only-token", } steps = [step for step in workflow["jobs"]["signoff"]["steps"] if "run" in step] for step in steps: @@ -88,6 +91,112 @@ def check_dco( return result +@pytest.fixture +def github_api(tmp_path: Path, git_env: dict[str, str]): + """Only GitHub metadata is substituted; the shipped shell and Git are real.""" + bin_dir = tmp_path / "bin" + bin_dir.mkdir() + calls = tmp_path / "api-calls.txt" + executable = bin_dir / "gh" + executable.write_text( + f"#!{sys.executable}\n" + "import os, sys\n" + "from pathlib import Path\n" + "with Path(os.environ['DCO_TEST_CALLS']).open('a') as log:\n" + " log.write(' '.join(sys.argv[1:]) + '\\n')\n" + "print(os.environ.get('DCO_TEST_METADATA', '{}'))\n" + "sys.exit(int(os.environ.get('DCO_TEST_API_EXIT', '0')))\n", + encoding="utf-8", + ) + executable.chmod(0o755) + env = { + **git_env, "PATH": f"{bin_dir}{os.pathsep}{git_env['PATH']}", + "GITHUB_REPOSITORY": "qualification/dco", + "DCO_TEST_CALLS": str(calls), + } + return env, calls + + +def github_merge(repo: Path, env: dict[str, str], *, unsigned_topic: bool = False) -> str: + git(repo, env, "checkout", "-b", "topic") + commit(repo, env, "Topic contribution", signed=not unsigned_topic) + git(repo, env, "checkout", "contribution") + commit(repo, env, "Mainline contribution") + git(repo, {**env, "GIT_COMMITTER_NAME": "GitHub", "GIT_COMMITTER_EMAIL": "noreply@github.com"}, + "merge", "--no-ff", "topic", "-m", "Platform integration merge") + return git(repo, env, "rev-parse", "HEAD") + + +def verified_merge_record(repo: Path, env: dict[str, str], sha: str) -> dict: + return { + "sha": sha, "committer": {"login": "web-flow"}, + "commit": { + "committer": {"name": "GitHub", "email": "noreply@github.com"}, + "verification": {"verified": True, "reason": "valid"}, + }, + "parents": [{"sha": parent} for parent in git(repo, env, "show", "-s", "--format=%P", sha).split()], + } + + +@pytest.mark.parametrize("unsigned_topic", [False, True]) +def test_verified_platform_merge_does_not_exempt_its_contributions(history, github_api, unsigned_topic): + runner, old_base, _ = history + env, calls = github_api + head = github_merge(runner, env, unsigned_topic=unsigned_topic) + metadata = verified_merge_record(runner, env, head) + result = check_dco(runner, {**env, "DCO_TEST_METADATA": json.dumps(metadata)}, old_base, head) + assert (result.returncode == 0) == (not unsigned_topic), result.stdout + result.stderr + assert f"Verified GitHub-generated merge {head}" in result.stdout + assert f"Commit {head} is missing" not in result.stdout + if unsigned_topic: + topic = git(runner, env, "rev-parse", "topic") + assert f"Commit {topic} is missing" in result.stdout + assert calls.read_text().strip() == f"api repos/qualification/dco/commits/{head}" + + +@pytest.mark.parametrize("mutation", ["unsigned", "other-signer", "wrong-sha", "wrong-parent", "missing-verification"]) +def test_merge_identity_without_exact_verified_provenance_is_not_exempt(history, github_api, mutation): + runner, old_base, _ = history + env, _ = github_api + head = github_merge(runner, env) + metadata = verified_merge_record(runner, env, head) + if mutation == "unsigned": + metadata["commit"]["verification"] = {"verified": False, "reason": "unsigned"} + elif mutation == "other-signer": + metadata["committer"]["login"] = "contributor" + elif mutation == "wrong-sha": + metadata["sha"] = "0" * 40 + elif mutation == "wrong-parent": + metadata["parents"][0]["sha"] = "0" * 40 + else: + del metadata["commit"]["verification"] + result = check_dco(runner, {**env, "DCO_TEST_METADATA": json.dumps(metadata)}, old_base, head) + assert result.returncode != 0 + assert f"Commit {head} is missing" in result.stdout + + +def test_platform_provenance_api_failure_fails_closed_with_retry_guidance(history, github_api): + runner, old_base, _ = history + env, _ = github_api + head = github_merge(runner, env) + result = check_dco(runner, {**env, "DCO_TEST_API_EXIT": "1"}, old_base, head) + assert result.returncode != 0 + assert "Cannot verify GitHub merge provenance" in result.stdout + assert "retry the check" in result.stdout + + +def test_unsigned_single_parent_web_commit_still_requires_dco(history, github_api): + runner, old_base, _ = history + env, calls = github_api + head = commit(runner, {**env, "GIT_COMMITTER_NAME": "GitHub", "GIT_COMMITTER_EMAIL": "noreply@github.com"}, + "Web suggestion without DCO", signed=False) + metadata = verified_merge_record(runner, env, head) + result = check_dco(runner, {**env, "DCO_TEST_METADATA": json.dumps(metadata)}, old_base, head) + assert result.returncode != 0 + assert f"Commit {head} is missing" in result.stdout + assert not calls.exists() + + @pytest.mark.parametrize("change", ["code", "docs"]) def test_signed_pr_excludes_newer_upstream_commits(history, git_env, change): runner, old_base, upstream = history