diff --git a/.github/workflows/pr-orchestrator.yml b/.github/workflows/pr-orchestrator.yml index 06a1f71d..fd562591 100644 --- a/.github/workflows/pr-orchestrator.yml +++ b/.github/workflows/pr-orchestrator.yml @@ -116,9 +116,6 @@ jobs: VERIFY_ARGS=(--enforce-version-bump --payload-from-filesystem) if [ "${{ github.event_name }}" = "pull_request" ]; then BASE_REF="origin/${{ github.event.pull_request.base.ref }}" - if [ "${{ github.event.pull_request.base.ref }}" = "main" ]; then - VERIFY_ARGS=(--require-signature "${VERIFY_ARGS[@]}") - fi elif [ "${{ github.ref_name }}" = "main" ]; then VERIFY_ARGS=(--require-signature "${VERIFY_ARGS[@]}") fi diff --git a/.github/workflows/sign-modules-on-approval.yml b/.github/workflows/sign-modules-on-approval.yml new file mode 100644 index 00000000..222c3802 --- /dev/null +++ b/.github/workflows/sign-modules-on-approval.yml @@ -0,0 +1,84 @@ +# yaml-language-server: $schema=https://json.schemastore.org/github-workflow.json +# Sign changed bundled module manifests after a PR is approved (same-repo PRs only). +name: Sign modules on PR approval + +on: + pull_request_review: + types: [submitted] + +concurrency: + group: sign-modules-on-approval-${{ github.event.pull_request.number }} + cancel-in-progress: true + +jobs: + sign: + name: CI sign changed modules + if: | + github.event.review.state == 'approved' && + (github.event.pull_request.base.ref == 'dev' || github.event.pull_request.base.ref == 'main') && + github.event.pull_request.head.repo.full_name == github.repository + runs-on: ubuntu-latest + permissions: + contents: write + steps: + - name: Require module signing key secret + env: + SPECFACT_MODULE_PRIVATE_SIGN_KEY: ${{ secrets.SPECFACT_MODULE_PRIVATE_SIGN_KEY }} + run: | + if [ -z "${SPECFACT_MODULE_PRIVATE_SIGN_KEY}" ]; then + echo "::error::Missing or empty repository secret SPECFACT_MODULE_PRIVATE_SIGN_KEY. Configure it to enable approval-time signing." + exit 1 + fi + + - name: Checkout PR head + uses: actions/checkout@v4 + with: + fetch-depth: 0 + ref: ${{ github.event.pull_request.head.sha }} + persist-credentials: true + + - name: Fetch base branch for change detection + run: git fetch --no-tags origin "${{ github.event.pull_request.base.ref }}" + + - name: Set up Python + uses: actions/setup-python@v5 + with: + python-version: "3.12" + + - name: Install signer dependencies + run: | + python -m pip install --upgrade pip + python -m pip install pyyaml beartype icontract cryptography cffi + + - name: Sign changed module manifests + env: + SPECFACT_MODULE_PRIVATE_SIGN_KEY: ${{ secrets.SPECFACT_MODULE_PRIVATE_SIGN_KEY }} + SPECFACT_MODULE_PRIVATE_SIGN_KEY_PASSPHRASE: ${{ secrets.SPECFACT_MODULE_PRIVATE_SIGN_KEY_PASSPHRASE }} + run: | + BASE_REF="origin/${{ github.event.pull_request.base.ref }}" + python scripts/sign-modules.py \ + --changed-only \ + --base-ref "${BASE_REF}" \ + --bump-version patch \ + --payload-from-filesystem + + - name: Commit and push signed manifests + env: + HEAD_REF: ${{ github.event.pull_request.head.ref }} + run: | + git config user.name "github-actions[bot]" + git config user.email "41898282+github-actions[bot]@users.noreply.github.com" + if git diff --quiet; then + echo "No manifest changes to commit." + echo "## No signing changes" >> "${GITHUB_STEP_SUMMARY}" + exit 0 + fi + git add -u -- src/specfact_cli/modules modules + if git diff --cached --quiet; then + echo "No staged module manifest updates." + exit 0 + fi + git commit -m "chore(modules): ci sign changed modules [skip ci]" + git push origin "HEAD:${HEAD_REF}" + echo "## Signed manifests pushed" >> "${GITHUB_STEP_SUMMARY}" + echo "Updated \`module-package.yaml\` files were committed to \`${HEAD_REF}\`." >> "${GITHUB_STEP_SUMMARY}" diff --git a/.github/workflows/sign-modules.yml b/.github/workflows/sign-modules.yml index 29ab94c7..0c426e13 100644 --- a/.github/workflows/sign-modules.yml +++ b/.github/workflows/sign-modules.yml @@ -3,7 +3,27 @@ name: Module Signature Hardening on: - workflow_dispatch: {} + workflow_dispatch: + inputs: + base_branch: + description: Remote branch to compare for --changed-only (fetches origin/) + type: choice + options: + - dev + - main + default: dev + version_bump: + description: Auto-bump when module version is still unchanged from the base ref + type: choice + options: + - patch + - minor + - major + default: patch + resign_all_manifests: + description: Sign every bundled module-package.yaml (not only --changed-only vs base). Use when manifests match the base but lack signatures. + type: boolean + default: false push: branches: [dev, main] paths: @@ -13,6 +33,7 @@ on: - "scripts/sign-modules.py" - "scripts/verify-modules-signature.py" - ".github/workflows/sign-modules.yml" + - ".github/workflows/sign-modules-on-approval.yml" pull_request: branches: [dev, main] paths: @@ -22,6 +43,7 @@ on: - "scripts/sign-modules.py" - "scripts/verify-modules-signature.py" - ".github/workflows/sign-modules.yml" + - ".github/workflows/sign-modules-on-approval.yml" jobs: verify: @@ -35,6 +57,10 @@ jobs: with: fetch-depth: 0 + - name: Fetch workflow_dispatch comparison base + if: github.event_name == 'workflow_dispatch' + run: git fetch --no-tags origin "${{ github.event.inputs.base_branch }}" + - name: Set up Python uses: actions/setup-python@v5 with: @@ -48,17 +74,23 @@ jobs: - name: Verify bundled module signatures run: | BASE_REF="" + VERIFY_ARGS=(--enforce-version-bump --payload-from-filesystem) if [ "${{ github.event_name }}" = "pull_request" ]; then BASE_REF="origin/${{ github.event.pull_request.base.ref }}" + elif [ "${{ github.event_name }}" = "workflow_dispatch" ]; then + BASE_REF="origin/${{ github.event.inputs.base_branch }}" + elif [ "${{ github.ref_name }}" = "main" ] && [ "${{ github.event_name }}" != "workflow_dispatch" ]; then + VERIFY_ARGS=(--require-signature "${VERIFY_ARGS[@]}") fi if [ -n "$BASE_REF" ]; then - python scripts/verify-modules-signature.py --require-signature --enforce-version-bump --version-check-base "$BASE_REF" + python scripts/verify-modules-signature.py "${VERIFY_ARGS[@]}" --version-check-base "$BASE_REF" else - python scripts/verify-modules-signature.py --require-signature --enforce-version-bump + python scripts/verify-modules-signature.py "${VERIFY_ARGS[@]}" fi reproducibility: name: Assert signing reproducibility + if: github.event_name == 'push' runs-on: ubuntu-latest needs: [verify] permissions: @@ -100,3 +132,81 @@ jobs: git --no-pager diff --name-only -- src/specfact_cli/modules modules exit 1 fi + + sign-and-push: + name: Sign changed modules (manual dispatch) + if: github.event_name == 'workflow_dispatch' + runs-on: ubuntu-latest + needs: [verify] + permissions: + contents: write + steps: + - name: Require module signing key secret + env: + SPECFACT_MODULE_PRIVATE_SIGN_KEY: ${{ secrets.SPECFACT_MODULE_PRIVATE_SIGN_KEY }} + run: | + if [ -z "${SPECFACT_MODULE_PRIVATE_SIGN_KEY}" ]; then + echo "::error::Missing or empty repository secret SPECFACT_MODULE_PRIVATE_SIGN_KEY." + exit 1 + fi + + - name: Checkout branch + uses: actions/checkout@v4 + with: + fetch-depth: 0 + ref: ${{ github.ref }} + persist-credentials: true + + - name: Fetch comparison base + run: git fetch --no-tags origin "${{ github.event.inputs.base_branch }}" + + - name: Set up Python + uses: actions/setup-python@v5 + with: + python-version: "3.12" + + - name: Install signer dependencies + run: | + python -m pip install --upgrade pip + python -m pip install pyyaml beartype icontract cryptography cffi + + - name: Sign module manifests + env: + SPECFACT_MODULE_PRIVATE_SIGN_KEY: ${{ secrets.SPECFACT_MODULE_PRIVATE_SIGN_KEY }} + SPECFACT_MODULE_PRIVATE_SIGN_KEY_PASSPHRASE: ${{ secrets.SPECFACT_MODULE_PRIVATE_SIGN_KEY_PASSPHRASE }} + run: | + BASE_REF="origin/${{ github.event.inputs.base_branch }}" + BUMP="${{ github.event.inputs.version_bump }}" + if [ "${{ github.event.inputs.resign_all_manifests }}" = "true" ]; then + mapfile -t MANIFESTS < <(find src/specfact_cli/modules modules -name 'module-package.yaml' -type f 2>/dev/null | sort) + if [ "${#MANIFESTS[@]}" -eq 0 ]; then + echo "No module manifests found" + exit 0 + fi + python scripts/sign-modules.py --payload-from-filesystem "${MANIFESTS[@]}" + else + python scripts/sign-modules.py \ + --changed-only \ + --base-ref "${BASE_REF}" \ + --bump-version "${BUMP}" \ + --payload-from-filesystem + fi + + - name: Commit and push signed manifests + run: | + git config user.name "github-actions[bot]" + git config user.email "41898282+github-actions[bot]@users.noreply.github.com" + if git diff --quiet; then + echo "No manifest changes to commit." + echo "## No signing changes" >> "${GITHUB_STEP_SUMMARY}" + exit 0 + fi + git add -u -- src/specfact_cli/modules modules + if git diff --cached --quiet; then + echo "No staged module manifest updates." + exit 0 + fi + git commit -m "chore(modules): manual workflow_dispatch sign changed modules [skip ci]" + git push origin "HEAD:${GITHUB_REF_NAME}" + echo "## Signed manifests pushed" >> "${GITHUB_STEP_SUMMARY}" + echo "Branch: \`${GITHUB_REF_NAME}\` (base: \`origin/${{ github.event.inputs.base_branch }}\`, bump: \`${{ github.event.inputs.version_bump }}\`, resign_all: \`${{ github.event.inputs.resign_all_manifests }}\`)." >> "${GITHUB_STEP_SUMMARY}" diff --git a/CHANGELOG.md b/CHANGELOG.md index c8814ee6..f8d17837 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -14,6 +14,15 @@ All notable changes to this project will be documented in this file. ### Added +- **CI / modules**: `.github/workflows/sign-modules-on-approval.yml` — after an **approved** review on + same-repo PRs to `dev`/`main`, signs changed bundled modules with `scripts/sign-modules.py + --changed-only` and commits manifests to the PR branch (repository secrets + `SPECFACT_MODULE_PRIVATE_SIGN_KEY` / passphrase); documented in `docs/reference/module-security.md`. +- **CI / modules**: `sign-modules.yml` **workflow_dispatch** inputs (`base_branch`, `version_bump`, + `resign_all_manifests`) and a **`sign-and-push`** job; verify passes `--version-check-base` for manual + runs and fetches the selected base before verify; **reproducibility** runs on **push** only (not + `pull_request`) so unsigned PR heads do not fail CI; optional full-tree re-sign when + `--changed-only` would no-op. - **CI / release**: `scripts/check_local_version_ahead_of_pypi.py` and `hatch run check-pypi-ahead` — fail PR tests when `pyproject.toml` is not strictly newer than the latest `specfact-cli` on PyPI (same rule as publish; avoids silent “skipped publication” after merge to `main`). @@ -26,6 +35,14 @@ All notable changes to this project will be documented in this file. ### Fixed +- **CI / modules**: `sign-modules.yml` **Assert signing reproducibility** no longer runs on `pull_request` + (only on **push** to `dev`/`main`), fixing false failures when bundled manifests are checksum-only while + PR verify intentionally omits `--require-signature`. +- **CI module verify (PR vs `main` push)**: `pr-orchestrator` and `sign-modules` verify jobs no longer pass + `--require-signature` on `pull_request` (checksum + `--enforce-version-bump` only), avoiding false failures + when a manifest (e.g. `init`) has checksum but not yet `integrity.signature`. Pushes to **`main`** still run + strict `--require-signature` verification; sign bundled manifests before merging release PRs or post-merge + CI will fail. `sign-modules` verify now passes `--payload-from-filesystem` in line with the orchestrator. - **Pre-commit / CI parity**: `.pre-commit-config.yaml` markdown hooks now match the quality script glob by including `*.mdc`; `check_safe_change()` counts `openspec/changes/*` so OpenSpec delta Markdown is not treated as “safe-only” skips; `pr-orchestrator` verify job passes `--require-signature` only when the PR base (or push branch) diff --git a/docs/reference/module-security.md b/docs/reference/module-security.md index d44f98a1..f55c677a 100644 --- a/docs/reference/module-security.md +++ b/docs/reference/module-security.md @@ -50,6 +50,24 @@ Module packages carry **publisher** and **integrity** metadata so installation, - **Checksum-only** (default when `--require-signature` is omitted): still enforces payload checksums and, with `--enforce-version-bump`, version discipline — useful on feature branches and for dev-targeting CI without local signing keys. + - **GitHub Actions** (`pr-orchestrator.yml`, `sign-modules.yml`): pull-request jobs use + checksum-only verification (no `--require-signature`) so unsigned manifests can be reviewed before + merge; **pushes to `main`** run strict verification with `--require-signature`. + - **Approval-time signing** (`sign-modules-on-approval.yml`): on **approved** reviews for same-repo PRs + targeting **`dev` or `main`**, CI runs `scripts/sign-modules.py --changed-only` with repository secrets + (`SPECFACT_MODULE_PRIVATE_SIGN_KEY`, optional passphrase) and pushes updated `module-package.yaml` + files to the PR branch. That removes the need for a local signing key for routine agent/Copilot flows + as long as secrets are configured; fork PRs are skipped (push permission). If the workflow or + secrets are unavailable, sign bundled manifests before merging into `main` or the post-merge push + verify job will still fail. + - **Manual signing** (`sign-modules.yml` → **Run workflow**): choose the branch to update, then pick + **comparison base** (`dev` or `main`, i.e. `origin/` for `--changed-only`) and **version bump** + (`patch` / `minor` / `major`). Verification uses that same base as `--version-check-base` so + `workflow_dispatch` is not stuck on `HEAD~1` before the repair job runs. Enable **resign all manifests** + when trees match the base but signatures are still missing (unsigned file identical on both sides). + On `main`, strict `--require-signature` is skipped only for `workflow_dispatch` so you can recover + unsigned `main`. **Reproducibility** (re-sign, assert no diff) runs on **push** to `dev`/`main` only, + not on `pull_request`, so PRs stay green while manifests are still unsigned. - There is **no** `--allow-unsigned` on this verifier; that flag exists on **`sign-modules.py`** for explicit test-only signing without a key. - **Pre-commit** (this repo): when staged paths exist under `modules/` or `src/specfact_cli/modules/`, diff --git a/tests/unit/specfact_cli/registry/test_signing_artifacts.py b/tests/unit/specfact_cli/registry/test_signing_artifacts.py index 45feb6a4..e6e51ec1 100644 --- a/tests/unit/specfact_cli/registry/test_signing_artifacts.py +++ b/tests/unit/specfact_cli/registry/test_signing_artifacts.py @@ -435,6 +435,67 @@ def test_sign_modules_workflow_valid_yaml(): assert isinstance(data, dict) +def _sign_modules_on_block(workflow_root: dict[str, Any]) -> dict[str, Any]: + on_block = workflow_root.get("on") + if on_block is None: + on_block = cast(dict[object, Any], workflow_root).get(True) + assert isinstance(on_block, dict), "sign-modules workflow must define on: mappings" + return cast(dict[str, Any], on_block) + + +def _assert_workflow_dispatch_inputs(on_block: dict[str, Any]) -> None: + dispatch = on_block.get("workflow_dispatch") + assert isinstance(dispatch, dict), "workflow_dispatch must be configured with inputs" + inputs = dispatch.get("inputs") + assert isinstance(inputs, dict) + assert "base_branch" in inputs + assert "version_bump" in inputs + assert "resign_all_manifests" in inputs + + +def _assert_sign_and_push_job(workflow_root: dict[str, Any]) -> None: + jobs = workflow_root.get("jobs") + assert isinstance(jobs, dict) + sign_push = jobs.get("sign-and-push") + assert isinstance(sign_push, dict) + assert sign_push.get("if") == "github.event_name == 'workflow_dispatch'" + assert sign_push.get("needs") == ["verify"] + perms = sign_push.get("permissions") + assert isinstance(perms, dict) and perms.get("contents") == "write" + + +def _assert_sign_modules_dispatch_raw_content(raw: str) -> None: + assert "github.event.inputs.base_branch" in raw + assert "github.event.inputs.version_bump" in raw + assert "github.event.inputs.resign_all_manifests" in raw + assert "Fetch workflow_dispatch comparison base" in raw + assert 'elif [ "${{ github.event_name }}" = "workflow_dispatch" ]; then' in raw + assert "--changed-only" in raw + assert "chore(modules): manual workflow_dispatch sign changed modules" in raw + + +def test_sign_modules_workflow_dispatch_signs_changed_modules_and_pushes(): + """Manual workflow_dispatch SHALL offer base/bump inputs and a sign-and-push job.""" + if not SIGN_WORKFLOW.exists(): + pytest.skip("workflow not present") + data = yaml.safe_load(SIGN_WORKFLOW.read_text(encoding="utf-8")) + assert isinstance(data, dict) + workflow_root = cast(dict[str, Any], data) + on_block = _sign_modules_on_block(workflow_root) + _assert_workflow_dispatch_inputs(on_block) + _assert_sign_and_push_job(workflow_root) + _assert_sign_modules_dispatch_raw_content(SIGN_WORKFLOW.read_text(encoding="utf-8")) + + +def test_sign_modules_reproducibility_runs_only_on_push(): + """Re-sign diff check must not run on pull_request (unsigned manifests OK) or workflow_dispatch.""" + if not SIGN_WORKFLOW.exists(): + pytest.skip("workflow not present") + raw = SIGN_WORKFLOW.read_text(encoding="utf-8") + assert "name: Assert signing reproducibility" in raw + assert "if: github.event_name == 'push'" in raw + + def test_verify_modules_script_exists(): """Verification script SHALL exist for CI signature validation.""" assert VERIFY_PYTHON_SCRIPT.exists(), "scripts/verify-modules-signature.py must exist" @@ -505,7 +566,8 @@ def test_pr_orchestrator_contains_verify_module_signatures_job(): pytest.skip("pr-orchestrator workflow not present") content = PR_ORCHESTRATOR_WORKFLOW.read_text(encoding="utf-8") assert "verify-module-signatures" in content - assert "verify-modules-signature.py --require-signature" in content + assert "verify-modules-signature.py" in content + assert "--require-signature" in content assert "--enforce-version-bump" in content assert "SPECFACT_MODULE_PRIVATE_SIGN_KEY" in content assert "SPECFACT_MODULE_PRIVATE_SIGN_KEY_PASSPHRASE" in content diff --git a/tests/unit/workflows/test_sign_modules_on_approval.py b/tests/unit/workflows/test_sign_modules_on_approval.py new file mode 100644 index 00000000..c9bdd42c --- /dev/null +++ b/tests/unit/workflows/test_sign_modules_on_approval.py @@ -0,0 +1,67 @@ +"""Policy tests for sign-modules-on-approval workflow.""" + +from __future__ import annotations + +from pathlib import Path +from typing import Any, cast + +import yaml + + +REPO_ROOT = Path(__file__).resolve().parents[3] +WORKFLOW = REPO_ROOT / ".github" / "workflows" / "sign-modules-on-approval.yml" + + +def _load_yaml(path: Path) -> dict[str, Any]: + data = yaml.safe_load(path.read_text(encoding="utf-8")) + assert isinstance(data, dict), f"Expected mapping at {path}" + return cast(dict[str, Any], data) + + +def _workflow_on_block(workflow: dict[str, Any]) -> dict[str, Any]: + on_block = workflow.get("on") + if on_block is None: + on_block = cast(dict[object, Any], workflow).get(True) + assert isinstance(on_block, dict), "Workflow must define event mappings" + return cast(dict[str, Any], on_block) + + +def test_sign_modules_on_approval_workflow_exists() -> None: + assert WORKFLOW.is_file(), "sign-modules-on-approval.yml must exist" + + +def test_sign_modules_on_approval_trigger_and_guards() -> None: + workflow = _load_yaml(WORKFLOW) + on_block = _workflow_on_block(workflow) + review = on_block.get("pull_request_review") + assert isinstance(review, dict), "Expected pull_request_review trigger mapping" + assert review.get("types") == ["submitted"] + + jobs = workflow.get("jobs") + assert isinstance(jobs, dict) + sign_job = jobs.get("sign") + assert isinstance(sign_job, dict) + job_if = sign_job.get("if") + assert isinstance(job_if, str) + assert "github.event.review.state == 'approved'" in job_if + assert "github.event.pull_request.base.ref == 'dev'" in job_if + assert "github.event.pull_request.base.ref == 'main'" in job_if + assert "github.event.pull_request.head.repo.full_name == github.repository" in job_if + + perms = sign_job.get("permissions") + assert isinstance(perms, dict) + assert perms.get("contents") == "write" + + +def test_sign_modules_on_approval_runs_signer_with_changed_only_mode() -> None: + raw = WORKFLOW.read_text(encoding="utf-8") + assert "scripts/sign-modules.py" in raw + assert "--changed-only" in raw + assert "--bump-version patch" in raw + assert "--payload-from-filesystem" in raw + assert '--base-ref "${BASE_REF}"' in raw + assert "origin/${{ github.event.pull_request.base.ref }}" in raw + assert "SPECFACT_MODULE_PRIVATE_SIGN_KEY" in raw + assert "SPECFACT_MODULE_PRIVATE_SIGN_KEY_PASSPHRASE" in raw + assert "chore(modules): ci sign changed modules [skip ci]" in raw + assert 'git push origin "HEAD:${HEAD_REF}"' in raw