diff --git a/.github/workflows/build-test-release.yaml b/.github/workflows/build-test-release.yaml index 0712b81..6dc6f93 100644 --- a/.github/workflows/build-test-release.yaml +++ b/.github/workflows/build-test-release.yaml @@ -73,20 +73,11 @@ jobs: with: token: "${{ secrets.GITHUB_TOKEN }}" require_type: 'semver' + reject_development: 'true' require_github: 'true' # yamllint disable-line rule:line-length require_signed: 'ssh,gpg-unverifiable' # Cannot verify GPG without key - - name: 'Reject development tags' - if: steps.tag-validate.outputs.development_tag == 'true' - shell: bash - run: | - # Reject development tags - echo "Development tag pushed; aborting release workflow 🛑" - echo "Development tag pushed; aborting release workflow 🛑" \ - >> "$GITHUB_STEP_SUMMARY" - exit 1 - - name: 'Ensure draft release exists' id: 'ensure-release' shell: bash @@ -207,6 +198,12 @@ jobs: permissions: contents: read steps: + # Harden the runner used by this workflow + # yamllint disable-line rule:line-length + - uses: step-security/harden-runner@a90bcbc6539c36a85cdfeb73f7e2f433735f215b # v2.15.0 + with: + egress-policy: 'audit' + # yamllint disable-line rule:line-length - uses: actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd # v6.0.2 @@ -232,9 +229,9 @@ jobs: # yamllint disable-line rule:line-length uses: anchore/scan-action@7037fa011853d5a11690026fb85feee79f4c946c # v7.3.2 id: grype-sarif - # The first Grype audit should not be blocking and should never fail - # That way we always get the required artefact; the second job will - # exit with an error, as required (below) + # The first Grype scan should not abort the job on failure so that + # subsequent steps can collect artefacts and display human-readable + # results; the final check step will fail the job if needed continue-on-error: true with: sbom: "${{ steps.sbom.outputs.sbom_json_path }}" @@ -251,6 +248,8 @@ jobs: sbom: "${{ steps.sbom.outputs.sbom_json_path }}" output-format: "table" output-file: "grype-results.txt" + # Differs from build-test.yaml, which requires fixes to be applied + # Deliberately set to not block releases if/when tags are pushed fail-build: "false" - name: "Upload Grype scan results" @@ -268,10 +267,32 @@ jobs: if: always() run: | # Grype summary - echo "## Grype Summary" >> "$GITHUB_STEP_SUMMARY" - [ -f grype-results.txt ] && cat grype-results.txt \ - >> "$GITHUB_STEP_SUMMARY" || echo "No scan results available" \ - >> "$GITHUB_STEP_SUMMARY" + { + echo "## SBOM Summary" + echo "SBOM count: ${{ steps.sbom.outputs.component_count }}" + echo "Tool used: ${{ steps.sbom.outputs.dependency_manager }}" + echo "" + echo "## Grype Vulnerability Scan" + if [ -f grype-results.txt ]; then + cat grype-results.txt + else + echo "No scan results available" + fi + } >> "$GITHUB_STEP_SUMMARY" + if [ -f grype-results.txt ]; then + echo "--- Grype scan results ---" + cat grype-results.txt + fi + + - name: "Check Grype scan results" + if: steps.grype-sarif.outcome == 'failure' + run: | + # Check Grype scan results + echo "::error::Grype found vulnerabilities" \ + "at or above severity threshold" + echo "Review the Grype Summary above or download the" \ + "grype-scan-results artifact for details" + exit 1 test-pypi: name: 'Test PyPI Publishing' @@ -380,7 +401,8 @@ jobs: contents: write # IMPORTANT: needed for draft release promotion timeout-minutes: 2 outputs: - release_url: "${{ steps.promote-release.outputs.release_url }}" + # yamllint disable-line rule:line-length + release_url: "${{ steps.promote-release.outputs.release_url || steps.set-promoted-url.outputs.release_url }}" steps: # Harden the runner used by this workflow # yamllint disable-line rule:line-length @@ -419,6 +441,7 @@ jobs: latest: true - name: 'Set release URL for already promoted release' + id: 'set-promoted-url' if: steps.check-promoted.outputs.already_promoted == 'true' shell: bash env: diff --git a/.github/workflows/build-test.yaml b/.github/workflows/build-test.yaml index 96312d7..bda6418 100644 --- a/.github/workflows/build-test.yaml +++ b/.github/workflows/build-test.yaml @@ -8,6 +8,12 @@ name: 'Python Build/Test' # yamllint disable-line rule:truthy on: workflow_dispatch: + inputs: + clear_cache: + description: 'Clear all Python dependency caches' + type: boolean + default: false + required: false pull_request: types: [opened, reopened, edited, synchronize] branches: @@ -23,9 +29,39 @@ concurrency: group: "${{ github.workflow }}-${{ github.ref }}" cancel-in-progress: true -permissions: {} +permissions: + actions: write # Required for cache deletion when clear_cache is true jobs: + repository-metadata: + name: "Repository Metadata" + runs-on: ubuntu-latest + permissions: + contents: read + pull-requests: read + timeout-minutes: 5 + steps: + # yamllint disable-line rule:line-length + - uses: step-security/harden-runner@a90bcbc6539c36a85cdfeb73f7e2f433735f215b # v2.15.0 + with: + egress-policy: audit + + # yamllint disable-line rule:line-length + - uses: actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd # v6.0.2 + with: + fetch-depth: 0 + + - name: "Gather repository metadata" + id: repo-metadata + # yamllint disable-line rule:line-length + uses: lfreleng-actions/repository-metadata-action@ceabcd987d13d7bfefd2372e01eebb0ddac45956 # v0.2.0 + with: + github_token: ${{ secrets.GITHUB_TOKEN }} + github_summary: 'true' + gerrit_summary: 'false' + artifact_upload: 'true' + artifact_formats: 'json' + python-build: name: 'Python Build' runs-on: 'ubuntu-latest' @@ -35,23 +71,26 @@ jobs: artefact_path: "${{ steps.python-build.outputs.artefact_path }}" permissions: contents: read + actions: write # Required for cache deletion when clear_cache is true timeout-minutes: 12 env: GH_TOKEN: "${{ secrets.GITHUB_TOKEN }}" steps: # Harden the runner used by this workflow # yamllint disable-line rule:line-length - - uses: step-security/harden-runner@a90bcbc6539c36a85cdfeb73f7e2f433735f215b # v2.15.0 + - uses: step-security/harden-runner@a90bcbc6539c36a85cdfeb73f7e2f433735f215b # v2.15.0 with: egress-policy: 'audit' # yamllint disable-line rule:line-length - - uses: actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd # v6.0.2 + - uses: actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd # v6.0.2 - name: 'Build Python project' id: python-build # yamllint disable-line rule:line-length - uses: lfreleng-actions/python-build-action@8e55c263713b2d2c8347c1d267e75b2a886d570a # v1.0.3 + uses: lfreleng-actions/python-build-action@8e55c263713b2d2c8347c1d267e75b2a886d570a # v1.0.3 + with: + clear_cache: ${{ github.event.inputs.clear_cache || 'false' }} python-tests: name: 'Python Tests' @@ -67,16 +106,16 @@ jobs: steps: # Harden the runner used by this workflow # yamllint disable-line rule:line-length - - uses: step-security/harden-runner@a90bcbc6539c36a85cdfeb73f7e2f433735f215b # v2.15.0 + - uses: step-security/harden-runner@a90bcbc6539c36a85cdfeb73f7e2f433735f215b # v2.15.0 with: egress-policy: audit # yamllint disable-line rule:line-length - - uses: actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd # v6.0.2 + - uses: actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd # v6.0.2 - name: "Python tests [pytest] ${{ matrix.python-version }}" # yamllint disable-line rule:line-length - uses: lfreleng-actions/python-test-action@92d4110d44ebc18fa4575c6b00203ff67d01a1cb # v1.0.1 + uses: lfreleng-actions/python-test-action@92d4110d44ebc18fa4575c6b00203ff67d01a1cb # v1.0.1 with: python_version: ${{ matrix.python-version }} @@ -94,19 +133,18 @@ jobs: steps: # Harden the runner used by this workflow # yamllint disable-line rule:line-length - - uses: step-security/harden-runner@a90bcbc6539c36a85cdfeb73f7e2f433735f215b # v2.15.0 + - uses: step-security/harden-runner@a90bcbc6539c36a85cdfeb73f7e2f433735f215b # v2.15.0 with: egress-policy: 'audit' # yamllint disable-line rule:line-length - - uses: actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd # v6.0.2 + - uses: actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd # v6.0.2 - name: "Audit dependencies ${{ matrix.python-version }}" # yamllint disable-line rule:line-length - uses: lfreleng-actions/python-audit-action@ec8d84ca14c0413a2b2c6612a3e15b9803f9de75 # v0.2.5 + uses: lfreleng-actions/python-audit-action@ec8d84ca14c0413a2b2c6612a3e15b9803f9de75 # v0.2.5 with: python_version: "${{ matrix.python-version }}" - permit_fail: true # Allow failure for development packages not on PyPI sbom: name: 'Generate SBOM' @@ -137,14 +175,69 @@ jobs: sbom-cyclonedx.xml retention-days: 45 - - name: "Security scan with Grype" + - name: "Security scan with Grype (SARIF)" # yamllint disable-line rule:line-length uses: anchore/scan-action@7037fa011853d5a11690026fb85feee79f4c946c # v7.3.2 + id: grype-sarif + # The first Grype scan should not abort the job on failure so that + # subsequent steps can collect artefacts and display human-readable + # results; the final check step will fail the job if needed + continue-on-error: true with: sbom: "${{ steps.sbom.outputs.sbom_json_path }}" + output-format: "sarif" + output-file: "grype-results.sarif" + fail-build: "true" - - name: "Summary step" + - name: "Security scan with Grype (Text/Table)" + # yamllint disable-line rule:line-length + uses: anchore/scan-action@7037fa011853d5a11690026fb85feee79f4c946c # v7.3.2 + id: grype-table + if: always() + with: + sbom: "${{ steps.sbom.outputs.sbom_json_path }}" + output-format: "table" + output-file: "grype-results.txt" + fail-build: "false" + + - name: "Upload Grype scan results" + # yamllint disable-line rule:line-length + uses: actions/upload-artifact@b7c566a772e6b6bfb58ed0dc250532a479d7789f # v6.0.0 + if: always() + with: + name: grype-scan-results + path: | + grype-results.sarif + grype-results.txt + retention-days: 90 + + - name: "Grype summary" + if: always() + run: | + # Grype summary + { + echo "## SBOM Summary" + echo "SBOM count: ${{ steps.sbom.outputs.component_count }}" + echo "Tool used: ${{ steps.sbom.outputs.dependency_manager }}" + echo "" + echo "## Grype Vulnerability Scan" + if [ -f grype-results.txt ]; then + cat grype-results.txt + else + echo "No scan results available" + fi + } >> "$GITHUB_STEP_SUMMARY" + if [ -f grype-results.txt ]; then + echo "--- Grype scan results ---" + cat grype-results.txt + fi + + - name: "Check Grype scan results" + if: steps.grype-sarif.outcome == 'failure' run: | - # Summary step - echo "SBOM count: ${{ steps.sbom.outputs.component_count }}" - echo "Tool used: ${{ steps.sbom.outputs.dependency_manager }}" + # Check Grype scan results + echo "::error::Grype found vulnerabilities" \ + "at or above severity threshold" + echo "Review the Grype Summary above or download the" \ + "grype-scan-results artifact for details" + exit 1 diff --git a/src/github2gerrit/core.py b/src/github2gerrit/core.py index dab6305..fa818c9 100644 --- a/src/github2gerrit/core.py +++ b/src/github2gerrit/core.py @@ -3424,7 +3424,7 @@ def _maybe_reuse_change_id(pr_str: str) -> str: ) if ( not sync_all_prs - and gh.event_name == "pull_request_target" + and gh.event_name in ("pull_request", "pull_request_target") and gh.event_action in ("reopened", "synchronize") ): try: @@ -5518,8 +5518,9 @@ def _close_pull_request_if_required( self, gh: GitHubContext, ) -> None: - """Close the PR if policy requires (pull_request_target events). + """Close the PR if policy requires (pull_request events). + Supports both ``pull_request`` and ``pull_request_target`` triggers. When PRESERVE_GITHUB_PRS is true, skip closing PRs (useful for testing). """ # Respect PRESERVE_GITHUB_PRS to avoid closing PRs during tests @@ -5530,9 +5531,9 @@ def _close_pull_request_if_required( gh.pr_number, ) return - # The current shell action closes PR on pull_request_target events. - if gh.event_name != "pull_request_target": - log.debug("Event is not pull_request_target; not closing PR") + # Close PRs on pull_request or pull_request_target events. + if gh.event_name not in ("pull_request", "pull_request_target"): + log.debug("Event is not a pull_request event; not closing PR") return log.info("Closing PR #%s", gh.pr_number) try: diff --git a/src/github2gerrit/models.py b/src/github2gerrit/models.py index f4f06b3..4f54eeb 100644 --- a/src/github2gerrit/models.py +++ b/src/github2gerrit/models.py @@ -117,10 +117,15 @@ class GitHubContext: def get_operation_mode(self) -> PROperationMode: """Determine the operation mode based on event type and action. + Supports both ``pull_request`` and ``pull_request_target`` triggers. + Using ``pull_request`` is preferred for security (avoids granting + secrets to untrusted fork code), while ``pull_request_target`` is + accepted for backward compatibility. + Returns: PROperationMode enum indicating the type of operation """ - if self.event_name != "pull_request_target": + if self.event_name not in ("pull_request", "pull_request_target"): return PROperationMode.UNKNOWN action = self.event_action.lower() if self.event_action else "" diff --git a/tests/test_core_close_pr_policy.py b/tests/test_core_close_pr_policy.py index 5b51923..0caa7b9 100644 --- a/tests/test_core_close_pr_policy.py +++ b/tests/test_core_close_pr_policy.py @@ -58,16 +58,16 @@ def _fail(*args: Any, **kwargs: Any) -> Any: orch._close_pull_request_if_required(gh) -def test_close_pr_not_invoked_for_non_target_event( +def test_close_pr_not_invoked_for_non_pr_event( tmp_path: Path, monkeypatch: pytest.MonkeyPatch ) -> None: # Ensure preservation is disabled so only event controls behavior monkeypatch.delenv("PRESERVE_GITHUB_PRS", raising=False) - # Non-target event should not attempt to close + # Non-PR events (e.g. workflow_dispatch, push) should not attempt to close def _fail(*args: Any, **kwargs: Any) -> Any: raise AssertionError( - "GitHub API should not be called for non-target events" + "GitHub API should not be called for non-PR events" ) monkeypatch.setattr("github2gerrit.core.build_client", _fail) @@ -75,12 +75,66 @@ def _fail(*args: Any, **kwargs: Any) -> Any: monkeypatch.setattr("github2gerrit.core.get_pull", _fail) monkeypatch.setattr("github2gerrit.core.close_pr", _fail) + orch = Orchestrator(workspace=tmp_path) + gh = _gh_ctx(event_name="workflow_dispatch", pr_number=42) + + # Act: should no-op for non-PR events + orch._close_pull_request_if_required(gh) + + +def test_close_pr_invoked_for_pull_request_event( + tmp_path: Path, monkeypatch: pytest.MonkeyPatch +) -> None: + """Verify that pull_request events (not just pull_request_target) close PRs. + + The code now supports both ``pull_request`` and ``pull_request_target`` + triggers. Using ``pull_request`` is preferred for security. + """ + # Ensure preservation is disabled so closing is allowed + monkeypatch.delenv("PRESERVE_GITHUB_PRS", raising=False) + + calls: dict[str, Any] = {} + + class DummyClient: + pass + + class DummyRepo: + pass + + class DummyPR: + def __init__(self, number: int) -> None: + self.number = number + self.closed_state: str | None = None + + # Patch the GitHub helper functions used by the close path + monkeypatch.setattr("github2gerrit.core.build_client", DummyClient) + monkeypatch.setattr( + "github2gerrit.core.get_repo_from_env", lambda _c: DummyRepo() + ) + + def _get_pull(_repo: DummyRepo, number: int) -> DummyPR: + calls["get_pull_number"] = number + return DummyPR(number) + + def _close_pr(pr: DummyPR, *, comment: str | None = None) -> None: + calls["closed_pr_number"] = pr.number + calls["comment"] = comment + pr.closed_state = "closed" + + monkeypatch.setattr("github2gerrit.core.get_pull", _get_pull) + monkeypatch.setattr("github2gerrit.core.close_pr", _close_pr) + orch = Orchestrator(workspace=tmp_path) gh = _gh_ctx(event_name="pull_request", pr_number=42) - # Act: should no-op + # Act: pull_request events should now close the PR orch._close_pull_request_if_required(gh) + # Assert + assert calls.get("get_pull_number") == 42 + assert calls.get("closed_pr_number") == 42 + assert calls.get("comment") == "Auto-closing pull request" + def test_close_pr_invoked_for_pull_request_target_event( tmp_path: Path, monkeypatch: pytest.MonkeyPatch