Skip to content

Refactor: Workflow security and trigger updates #133

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
ModeSevenIndustrialSolutions wants to merge 4 commits into from
Closed

Refactor: Workflow security and trigger updates #133

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
2abc090
Fix: Improve Grype scan output and enforce failure on findings
ModeSevenIndustrialSolutions Feb 19, 2026
17d7162
Feat: Support pull_request trigger alongside pull_request_target
ModeSevenIndustrialSolutions Feb 19, 2026
c6644ba
Feat: Add deployment template workflows for GitHub2Gerrit
ModeSevenIndustrialSolutions Feb 19, 2026
b3721f7
Chore: Remove old reusable workflow and add metadata job comments
ModeSevenIndustrialSolutions Feb 19, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
59 changes: 41 additions & 18 deletions .github/workflows/build-test-release.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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

Expand All @@ -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 }}"
Expand All @@ -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"
Expand All @@ -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
Comment on lines +290 to +295
Copy link

Copilot AI Feb 25, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The comments state the table scan is set to not block releases, but the workflow still fails the sbom job when steps.grype-sarif.outcome == 'failure' (and the SARIF scan has fail-build: "true"). As written, vulnerability findings will block tag-based releases. Either remove/relax this check in the release workflow, or set the SARIF scan to not fail the step (e.g., fail-build: "false") if releases should proceed.

Suggested change
# 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
# Check Grype scan results (non-blocking)
echo "::warning::Grype found vulnerabilities at or above severity threshold."
echo "Review the Grype Summary above or download the grype-scan-results artifact for details."

Copilot uses AI. Check for mistakes.

test-pypi:
name: 'Test PyPI Publishing'
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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:
Expand Down
125 changes: 109 additions & 16 deletions .github/workflows/build-test.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand All @@ -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
Comment on lines +32 to +33
Copy link

Copilot AI Feb 25, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Workflow-level permissions: actions: write is unnecessary here because every job defines its own permissions block (and python-build already requests actions: write). Keeping write permissions at the workflow level increases the chance future jobs inherit broader rights unintentionally; consider removing the top-level permissions block or setting it to {} and relying on job-scoped permissions only.

Suggested change
permissions:
actions: write # Required for cache deletion when clear_cache is true
permissions: {}

Copilot uses AI. Check for mistakes.

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'
Expand All @@ -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'
Expand All @@ -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 }}

Expand All @@ -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 }}"
Copy link

Copilot AI Feb 25, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

python-audit no longer sets permit_fail: true for lfreleng-actions/python-audit-action. This is a behavior change from the previous configuration (which explicitly allowed audit failures for dev/non-PyPI packages) and may cause CI to start failing on projects that previously passed. If the intent is still to allow non-blocking audits in this workflow, reintroduce permit_fail: true (or the action’s current equivalent).

Suggested change
python_version: "${{ matrix.python-version }}"
python_version: "${{ matrix.python-version }}"
permit_fail: true

Copilot uses AI. Check for mistakes.
permit_fail: true # Allow failure for development packages not on PyPI

sbom:
name: 'Generate SBOM'
Expand Down Expand Up @@ -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"
Comment on lines +178 to +201
Copy link

Copilot AI Feb 18, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The Grype scan no longer fails the workflow on vulnerabilities: the SARIF scan sets fail-build: "true" but also continue-on-error: true, and the follow-up table scan sets fail-build: "false". As written, this job will always succeed even when Grype finds findings, which is a security regression vs a blocking scan. Either remove continue-on-error from the blocking scan, or add an explicit failing step based on steps.grype-sarif.outcome/exit code (and keep SARIF upload non-blocking separately).

Copilot uses AI. Check for mistakes.
Copy link
Contributor Author

@ModeSevenIndustrialSolutions ModeSevenIndustrialSolutions Feb 25, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This has been addressed. The current PR now includes a "Check Grype scan results" step that checks steps.grype-sarif.outcome == 'failure' and exits with error code 1. The continue-on-error: true on the SARIF step allows subsequent steps (table output, artifact upload, summary) to complete before the explicit failure step fires. This is the intended pattern.


- 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
11 changes: 6 additions & 5 deletions src/github2gerrit/core.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand Down Expand Up @@ -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
Expand All @@ -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:
Expand Down
7 changes: 6 additions & 1 deletion src/github2gerrit/models.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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 ""
Expand Down
Loading
Loading