diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 3af39697e..f66395c62 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -30,7 +30,10 @@ jobs: - name: Checkout uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1 with: + ref: ${{ github.event.pull_request.head.sha || github.sha }} persist-credentials: false + - name: Verify exact source head + run: test "$(git rev-parse HEAD)" = "${{ github.event.pull_request.head.sha || github.sha }}" - name: Set up Python uses: actions/setup-python@5fda3b95a4ea91299a34e894583c3862153e4b97 # v7.0.0 with: @@ -56,7 +59,10 @@ jobs: - name: Checkout uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1 with: + ref: ${{ github.event.pull_request.head.sha || github.sha }} persist-credentials: false + - name: Verify exact source head + run: test "$(git rev-parse HEAD)" = "${{ github.event.pull_request.head.sha || github.sha }}" - name: Set up Python uses: actions/setup-python@5fda3b95a4ea91299a34e894583c3862153e4b97 # v7.0.0 with: @@ -94,7 +100,10 @@ jobs: - name: Checkout uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1 with: + ref: ${{ github.event.pull_request.head.sha || github.sha }} persist-credentials: false + - name: Verify exact source head + run: test "$(git rev-parse HEAD)" = "${{ github.event.pull_request.head.sha || github.sha }}" - name: Validate Compose configuration run: docker compose config >/dev/null - name: Build component image diff --git a/.github/workflows/hourly-maintenance.yml b/.github/workflows/hourly-maintenance.yml index c141f4693..d273fa2fd 100644 --- a/.github/workflows/hourly-maintenance.yml +++ b/.github/workflows/hourly-maintenance.yml @@ -15,21 +15,17 @@ permissions: jobs: review-fix: name: Repair actionable pull-request feedback - permissions: - actions: write - contents: read - issues: write - pull-requests: read - statuses: read - uses: ContextualWisdomLab/.github/.github/workflows/pr-review-fix-scheduler.yml@5983b41ace75040c1d81818171ca7d0f3653254e # main + uses: ContextualWisdomLab/.github/.github/workflows/pr-review-fix-scheduler.yml@355d9e545fd971638066e90654fdfaa105431968 # prerequisite .github#782 with: target_repository: ContextualWisdomLab/pg-llm-batch base_branch: main max_prs: "100" max_dispatches: "1" retry_hours: "1" - canonical_ref: 5983b41ace75040c1d81818171ca7d0f3653254e - secrets: inherit + canonical_ref: 355d9e545fd971638066e90654fdfaa105431968 + secrets: + PR_REVIEW_MERGE_TOKEN: ${{ secrets.PR_REVIEW_MERGE_TOKEN }} + OPENCODE_APPROVE_TOKEN: ${{ secrets.OPENCODE_APPROVE_TOKEN }} review-merge: name: Revalidate and merge eligible pull requests diff --git a/CHANGELOG.md b/CHANGELOG.md index 83fd56b75..6db1e1e36 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -69,6 +69,11 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Changed +- Narrowed the hourly review-repair caller to the immutable central OpenCode + scheduler prerequisite and only the established `PR_REVIEW_MERGE_TOKEN` and + `OPENCODE_APPROVE_TOKEN` secrets; the caller no longer elevates its generated + GitHub token or inherits unrelated secrets, while the independent merge plane + remains unchanged. - Migrated package licensing to PEP 639 with an SPDX `Apache-2.0` expression, explicit `LICENSE` and `NOTICE` files, and a compatible setuptools backend floor so built artifacts expose normalized legal metadata without warnings. diff --git a/docs/adr/0013-hourly-maintenance-credential-boundary.md b/docs/adr/0013-hourly-maintenance-credential-boundary.md new file mode 100644 index 000000000..f00f0bb93 --- /dev/null +++ b/docs/adr/0013-hourly-maintenance-credential-boundary.md @@ -0,0 +1,82 @@ +# ADR 0013: Hourly maintenance uses explicit scheduler credentials + +- **Status:** Proposed +- **Date:** 2026-08-07 +- **Decision owners:** ContextualWisdomLab maintainers + +## Context + +`pg-llm-batch` already runs an hourly pull-request maintenance workflow. Its +review-repair job pins an older central scheduler revision, grants write +permissions to the workflow-generated GitHub token, and forwards all repository +secrets through `secrets: inherit`. + +The central scheduler hardening in `ContextualWisdomLab/.github#782` separates +model authentication from GitHub mutation authority, binds privileged source to +an immutable called-workflow SHA, keeps `github.token` read-only, and permits +repository mutation only through the established explicitly mapped +`PR_REVIEW_MERGE_TOKEN` or `OPENCODE_APPROVE_TOKEN`. OpenCode model execution +continues to use `NVIDIA_NIM_API_KEY` inside the separately reviewed worker and +never uses `COPILOT_GITHUB_TOKEN`. + +The merge scheduler is a distinct review and merge plane. This change must not +alter its credential chain, permissions, or merge policy while repairing the +review-fix caller. + +## Decision + +The hourly `review-fix` job will: + +1. pin the central review-fix scheduler to the exact reviewed prerequisite SHA; +2. pass only `PR_REVIEW_MERGE_TOKEN` and `OPENCODE_APPROVE_TOKEN` explicitly; +3. remove job-level GitHub-token write permissions; +4. avoid `secrets: inherit`; +5. retain the existing one-hour cadence, one-dispatch bound, and single-flight + concurrency; and +6. keep the independent `review-merge` job byte-stable except for unavoidable + formatting generated by the same edit. + +The feature PR remains Draft and must not merge until the central prerequisite +is protected on `main`. Before promotion, its temporary prerequisite SHA must be +replaced by the resulting protected merge SHA and every exact-head check must be +rerun. + +## Consequences + +### Positive + +- The workflow-generated GitHub token stays read-only. +- Product callers no longer expose unrelated repository secrets to the reusable + repair scheduler. +- Standalone and MSA deployments share the same central scheduler contract. +- The model credential remains isolated inside the OpenCode worker. +- The existing reviewer and merge credential systems are not rewritten. + +### Trade-offs + +- Missing established scheduler credentials now cause an explicit fail-closed + run rather than a GitHub-token fallback. +- The caller depends on the central prerequisite merging before it can be + promoted. +- Operators must provision the two existing scheduler secrets where + cross-repository repair is required. + +## Verification + +Permanent tests require the exact central scheduler SHA, explicit secret +mapping, absence of review-fix token elevation and inheritance, and byte-stable +merge-plane policy. The complete repository CI, security, coverage, packaging, +and independent-review gates remain mandatory on the final protected-main base. + +## References + +GitHub. (n.d.). *Controlling permissions for GITHUB_TOKEN*. GitHub Docs. +Retrieved August 7, 2026, from +https://docs.github.com/en/actions/security-for-github-actions/security-guides/automatic-token-authentication#modifying-the-permissions-for-the-github_token + +GitHub. (n.d.). *Reusing workflows*. GitHub Docs. Retrieved August 7, 2026, from +https://docs.github.com/en/actions/how-tos/reuse-automations/reuse-workflows + +GitHub. (n.d.). *Use secrets in GitHub Actions*. GitHub Docs. Retrieved August +7, 2026, from +https://docs.github.com/en/actions/security-for-github-actions/security-guides/using-secrets-in-github-actions diff --git a/docs/superpowers/plans/2026-08-07-hourly-maintenance-credential-boundary.md b/docs/superpowers/plans/2026-08-07-hourly-maintenance-credential-boundary.md new file mode 100644 index 000000000..35ccc2758 --- /dev/null +++ b/docs/superpowers/plans/2026-08-07-hourly-maintenance-credential-boundary.md @@ -0,0 +1,65 @@ +# Hourly Maintenance Credential Boundary Implementation Plan + +> **Required workflow:** Use test-driven development and verification before +> completion. Do not add a temporary write-capable workflow. + +## Goal + +Migrate the `pg-llm-batch` hourly review-repair caller to the hardened central +scheduler contract while preserving the independent merge plane. + +## Dependency + +The implementation is stacked logically on `ContextualWisdomLab/.github#782`. +The current prerequisite SHA is used only for Draft verification. Final promotion +requires the protected merge SHA from central `main` and fresh exact-head gates. + +## Task 1: Prove the current caller is unsafe + +- [x] Add a focused test requiring the exact current central scheduler SHA. +- [x] Require explicit mapping of only the two established scheduler secrets. +- [x] Reject job-level GitHub-token write permissions in the review-fix job. +- [x] Reject `secrets: inherit` in the review-fix job. +- [x] Preserve the existing independent review-merge contract. +- [x] Record RED CI `31172887367` on test-only head + `df7a51333684ee6230a92696f9180cf39bbba5f5`. Its two credential-boundary + failures are the intended RED evidence; the other failures came from a + stale synthetic merge base and were fixed independently on protected + `main`, so they are not reused as feature evidence. + +## Task 2: Implement the bounded caller migration + +- [x] Pin `pr-review-fix-scheduler.yml` to current `.github#782` head + `355d9e545fd971638066e90654fdfaa105431968` for Draft verification. +- [x] Update `canonical_ref` to the same immutable SHA. +- [x] Remove the review-fix job permission elevation. +- [x] Replace inherited secrets with explicit scheduler secret mapping. +- [x] Keep cadence, retry floor, dispatch bound, and concurrency unchanged. +- [x] Keep the merge scheduler pin, permissions, inputs, and secret inheritance + unchanged. + +## Task 3: Document the operator and trust boundary + +- [x] Add ADR 0013 with GitHub primary-documentation references in APA 7 form. +- [x] Keep the existing secret names as the operator contract; no new credential + or model-secret setup is introduced by this caller-only migration. +- [x] Record the change under `CHANGELOG.md` Unreleased. + +## Task 4: Verify and promote + +- [ ] Run focused workflow-contract tests on the exact source head. +- [ ] Run the complete non-integration suite on the exact source head. +- [ ] Require Ruff and 100% production statement/branch coverage on the exact + source head. +- [ ] Require 100% production docstring coverage on the exact source head. +- [ ] Require lock freshness, package builds, Compose validation, and both + container builds on the exact source head. +- [ ] Require current-head security, SAST, dependency, SBOM, and review gates. +- [ ] After central #782 merges, replace the temporary SHA with its protected + merge SHA and rerun every gate. +- [ ] Merge only with zero unresolved valid findings and a qualifying independent + non-author approval. + +The current pull-request CI may validate GitHub's synthetic merge commit rather +than the source head. Such runs are useful regression evidence but do not satisfy +the exact-source-head acceptance items above. diff --git a/tests/bounded_response_double.py b/tests/bounded_response_double.py new file mode 100644 index 000000000..bc2974cd9 --- /dev/null +++ b/tests/bounded_response_double.py @@ -0,0 +1,36 @@ +# SPDX-License-Identifier: Apache-2.0 +"""Shared bounded-stream response doubles for provider-facing tests.""" + +from __future__ import annotations + +import json +from collections.abc import AsyncIterator, Mapping +from typing import Any + + +class BoundedJsonByteStream: + """Expose one canonical JSON object through aiohttp-style byte chunks.""" + + def __init__(self, payload: Mapping[str, Any]) -> None: + """Serialize one mapping once so tests exercise the production byte boundary.""" + self.payload_bytes = json.dumps( + dict(payload), + ensure_ascii=False, + separators=(",", ":"), + ).encode("utf-8") + self.requested_sizes: list[int] = [] + + async def iter_chunked(self, size: int) -> AsyncIterator[bytes]: + """Yield bounded chunks no larger than the requested positive size.""" + if isinstance(size, bool) or not isinstance(size, int) or size <= 0: + raise AssertionError("bounded response chunk size must be a positive integer") + self.requested_sizes.append(size) + for offset in range(0, len(self.payload_bytes), size): + yield self.payload_bytes[offset : offset + size] + + +def bind_bounded_json_response(response: Any, payload: Mapping[str, Any]) -> None: + """Attach deterministic aiohttp-compatible content metadata to a response double.""" + content = BoundedJsonByteStream(payload) + response.content = content + response.content_length = len(content.payload_bytes) diff --git a/tests/test_hourly_maintenance_credential_boundary.py b/tests/test_hourly_maintenance_credential_boundary.py new file mode 100644 index 000000000..1646cac22 --- /dev/null +++ b/tests/test_hourly_maintenance_credential_boundary.py @@ -0,0 +1,68 @@ +# SPDX-License-Identifier: Apache-2.0 +"""Contracts for the hourly maintenance scheduler credential boundary.""" + +from __future__ import annotations + +from pathlib import Path + + +WORKFLOW_PATH = Path(".github/workflows/hourly-maintenance.yml") +FIX_SCHEDULER_SHA = "355d9e545fd971638066e90654fdfaa105431968" +MERGE_SCHEDULER_SHA = "5983b41ace75040c1d81818171ca7d0f3653254e" + + +def _job_block(workflow: str, job_name: str, next_job_name: str | None) -> str: + """Return one exact top-level job block from the hourly workflow.""" + start = workflow.index(f" {job_name}:\n") + if next_job_name is None: + return workflow[start:] + end = workflow.index(f" {next_job_name}:\n", start) + return workflow[start:end] + + +def test_review_fix_uses_immutable_current_central_scheduler() -> None: + """The repair job pins the reviewed central scheduler prerequisite exactly.""" + workflow = WORKFLOW_PATH.read_text(encoding="utf-8") + review_fix = _job_block(workflow, "review-fix", "review-merge") + + assert ( + "uses: ContextualWisdomLab/.github/.github/workflows/" + f"pr-review-fix-scheduler.yml@{FIX_SCHEDULER_SHA}" + in review_fix + ) + assert f"canonical_ref: {FIX_SCHEDULER_SHA}" in review_fix + assert f"@{MERGE_SCHEDULER_SHA}" not in review_fix + + +def test_review_fix_never_elevates_or_inherits_workflow_token() -> None: + """Repair mutation uses only explicit established secrets, not GITHUB_TOKEN.""" + workflow = WORKFLOW_PATH.read_text(encoding="utf-8") + review_fix = _job_block(workflow, "review-fix", "review-merge") + + assert "\n permissions:\n" not in review_fix + assert "secrets: inherit" not in review_fix + assert ( + "PR_REVIEW_MERGE_TOKEN: ${{ secrets.PR_REVIEW_MERGE_TOKEN }}" + in review_fix + ) + assert ( + "OPENCODE_APPROVE_TOKEN: ${{ secrets.OPENCODE_APPROVE_TOKEN }}" + in review_fix + ) + assert "COPILOT_GITHUB_TOKEN" not in review_fix + assert "NVIDIA_NIM_API_KEY" not in review_fix + + +def test_merge_scheduler_contract_is_unchanged_by_repair_hardening() -> None: + """The independent merge plane retains its existing reviewed credential chain.""" + workflow = WORKFLOW_PATH.read_text(encoding="utf-8") + review_merge = _job_block(workflow, "review-merge", None) + + assert ( + "uses: ContextualWisdomLab/.github/.github/workflows/" + f"pr-review-merge-scheduler.yml@{MERGE_SCHEDULER_SHA}" + in review_merge + ) + assert "secrets: inherit" in review_merge + assert "merge_mode: direct_or_auto" in review_merge + assert "enable_auto_merge: true" in review_merge diff --git a/tests/test_remote_batch_identity_contract.py b/tests/test_remote_batch_identity_contract.py index 6fb9d2661..20c171c50 100644 --- a/tests/test_remote_batch_identity_contract.py +++ b/tests/test_remote_batch_identity_contract.py @@ -12,6 +12,8 @@ from pg_llm_batch.durable_client import DurableBatchAPIClient from pg_llm_batch.exceptions import GatewayError +from tests.bounded_response_double import bind_bounded_json_response + class _Response: """Expose one successful provider response through the aiohttp contract.""" @@ -19,6 +21,18 @@ class _Response: status = 200 headers: dict[str, str] = {} + def __init__(self) -> None: + payload = { + "id": "batch-other", + "status": "in_progress", + "request_counts": { + "total": 2, + "completed": 1, + "failed": 0, + }, + } + bind_bounded_json_response(self, payload) + async def json(self) -> dict[str, Any]: """Return a syntactically valid but identity-mismatched provider payload.""" return { diff --git a/tests/test_workflow_contracts.py b/tests/test_workflow_contracts.py index b130b234a..0a363561c 100644 --- a/tests/test_workflow_contracts.py +++ b/tests/test_workflow_contracts.py @@ -41,28 +41,44 @@ def test_ci_workflow_enforces_supported_versions_and_quality_gates() -> None: _assert_external_actions_are_pinned(workflow) +def test_ci_checks_out_and_verifies_the_exact_source_head_in_every_job() -> None: + """Every CI checkout must bind and verify the exact source head.""" + workflow = _read(".github/workflows/ci.yml") + exact_source_expression = "${{ github.event.pull_request.head.sha || github.sha }}" + checkout_count = workflow.count("uses: actions/checkout@") + + assert checkout_count > 0 + assert workflow.count(f"ref: {exact_source_expression}") == checkout_count + assert workflow.count("name: Verify exact source head") == checkout_count + assert workflow.count( + f'test "$(git rev-parse HEAD)" = "{exact_source_expression}"' + ) == checkout_count + + def test_hourly_workflow_repairs_revalidates_and_merges_pull_requests() -> None: workflow = _read(".github/workflows/hourly-maintenance.yml") - scheduler_sha = "5983b41ace75040c1d81818171ca7d0f3653254e" + review_fix_scheduler_sha = "355d9e545fd971638066e90654fdfaa105431968" + review_merge_scheduler_sha = "5983b41ace75040c1d81818171ca7d0f3653254e" assert 'cron: "17 * * * *"' in workflow assert "workflow_dispatch:" in workflow assert ( "uses: ContextualWisdomLab/.github/.github/workflows/" - "pr-review-fix-scheduler.yml@" + f"pr-review-fix-scheduler.yml@{review_fix_scheduler_sha}" ) in workflow assert "target_repository: ContextualWisdomLab/pg-llm-batch" in workflow assert 'retry_hours: "1"' in workflow - assert f"canonical_ref: {scheduler_sha}" in workflow + assert f"canonical_ref: {review_fix_scheduler_sha}" in workflow assert ( "uses: ContextualWisdomLab/.github/.github/workflows/" - "pr-review-merge-scheduler.yml@" + f"pr-review-merge-scheduler.yml@{review_merge_scheduler_sha}" ) in workflow assert "merge_mode: direct_or_auto" in workflow assert "trigger_reviews: true" in workflow assert "enable_auto_merge: true" in workflow assert "update_branches: true" in workflow - assert workflow.count(f"@{scheduler_sha}") == 2 + assert workflow.count(f"@{review_fix_scheduler_sha}") == 1 + assert workflow.count(f"@{review_merge_scheduler_sha}") == 1 _assert_external_actions_are_pinned(workflow)