feat: consolidate PR gates and harden security scans#2669
Open
DerekRoberts wants to merge 1 commit intomainfrom
Open
feat: consolidate PR gates and harden security scans#2669DerekRoberts wants to merge 1 commit intomainfrom
DerekRoberts wants to merge 1 commit intomainfrom
Conversation
DerekRoberts
force-pushed
the
feat/harden-ci-gates-v2
branch
from
17273b9 to
f9c1213
Compare
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
Comment on lines
+13
to
+52
| name: Backend | ||
| runs-on: ubuntu-24.04 | ||
| timeout-minutes: 5 | ||
| services: | ||
| postgres: | ||
| image: postgres | ||
| env: | ||
| POSTGRES_PASSWORD: default | ||
| options: >- | ||
| --health-cmd pg_isready | ||
| --health-interval 10s | ||
| --health-timeout 5s | ||
| --health-retries 5 | ||
| ports: | ||
| - 5432:5432 | ||
| steps: | ||
| - uses: actions/checkout@v6 | ||
| - uses: bcgov/action-test-and-analyse@v2 | ||
| env: | ||
| SONAR_TOKEN: ${{ secrets.SONAR_TOKEN_BACKEND }} | ||
| with: | ||
| commands: | | ||
| npm ci | ||
| npm run lint | ||
| npm run test:cov | ||
| dir: backend | ||
| node_version: "22" | ||
| sonar_args: > | ||
| -Dsonar.exclusions=**/coverage/**,**/node_modules/**,**/*spec.ts | ||
| -Dsonar.organization=bcgov-sonarcloud | ||
| -Dsonar.projectKey=quickstart-openshift_backend | ||
| -Dsonar.sources=src | ||
| -Dsonar.test.inclusions=**/*spec.ts | ||
| -Dsonar.javascript.lcov.reportPaths=./coverage/lcov.info | ||
| sonar_token: ${{ env.SONAR_TOKEN }} | ||
| dep_scan: warning | ||
| supply_scan: true | ||
| triggers: ('backend/') | ||
|
|
||
| frontend-tests: |
Comment on lines
+53
to
+80
| name: Frontend | ||
| runs-on: ubuntu-24.04 | ||
| timeout-minutes: 5 | ||
| steps: | ||
| - uses: actions/checkout@v6 | ||
| - uses: bcgov/action-test-and-analyse@v2 | ||
| env: | ||
| SONAR_TOKEN: ${{ secrets.SONAR_TOKEN_FRONTEND }} | ||
| with: | ||
| commands: | | ||
| npm ci | ||
| npm run lint | ||
| npm run test:cov | ||
| dir: frontend | ||
| node_version: "22" | ||
| sonar_args: >- | ||
| -Dsonar.exclusions=**/coverage/**,**/node_modules/**,**/*spec.ts | ||
| -Dsonar.organization=bcgov-sonarcloud | ||
| -Dsonar.projectKey=quickstart-openshift_frontend | ||
| -Dsonar.sources=src | ||
| -Dsonar.test.inclusions=**/*spec.ts | ||
| -Dsonar.javascript.lcov.reportPaths=./coverage/lcov.info | ||
| sonar_token: ${{ env.SONAR_TOKEN }} | ||
| dep_scan: warning | ||
| supply_scan: true | ||
| triggers: ('frontend/') | ||
|
|
||
| trivy: |
Contributor
There was a problem hiding this comment.
Pull request overview
This PR tightens merge gating by centralizing CI checks into the pr-open.yml workflow and making security scan failures (Trivy) block merges, addressing cases where PRs could merge despite failures in non-required workflows.
Changes:
- Added Backend/Frontend test jobs and a Trivy scan job to
pr-open.ymland included them in the unifiedPR Resultsgate. - Hardened Trivy scanning in
analysis.ymlby removingcontinue-on-errorand forcing non-zero exit on HIGH/CRITICAL findings. - Updated
Analysis Resultsto depend on Trivy and run withif: always().
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| .github/workflows/pr-open.yml | Adds backend/frontend unit+lint+coverage jobs and Trivy scan; makes PR Results depend on all PR checks. |
| .github/workflows/analysis.yml | Makes Trivy fail the workflow on findings and ensures Analysis Results reflects Trivy outcome. |
Comments suppressed due to low confidence (1)
.github/workflows/analysis.yml:119
analysis.ymlstill runs onpull_requestevents and includes backend/frontend tests + Trivy, but those same checks have now been added topr-open.ymland are included in the requiredPR Resultsgate. This will cause duplicate CI runs on every PR (extra time/cost) and can create confusion about which workflow is authoritative for merge gating. Consider removing thepull_requesttrigger fromanalysis.yml(keep it forpush/schedule), or gating these jobs to skip whengithub.event_name == 'pull_request'.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+59
to
+63
| SONAR_TOKEN_FRONTEND: ${{ secrets.SONAR_TOKEN_FRONTEND }} | ||
|
|
||
| results: | ||
| name: PR Results | ||
| needs: [builds, deploys, tests] | ||
| needs: [builds, deploys, tests, unit-tests] |
There was a problem hiding this comment.
The Postgres service uses the floating postgres image tag, which can change over time (including major-version upgrades) and make CI behavior non-deterministic. Pin the image to a specific major/minor (e.g., postgres:16 or similar) to keep PR test runs reproducible.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
This PR hardens the CI configuration to prevent merges when tests or security scans fail, addressing the issue where PRs (like #2667) were able to merge despite failing checks.
The Problem
The repository had fragmented workflows (
pr-open.ymlandanalysis.yml), each with their ownResultsjob. If only oneResultsjob was marked as required, the other workflow's failures would not block a merge. Additionally, Trivy scans were configured withcontinue-on-error: true, hiding security failures.The Fix
analysis.ymldirectly intopr-open.yml.PR Resultsjob now depends on ALL checks, including security.exit-code: 1and removedcontinue-on-errorfrom Trivy to ensure vulnerabilities block merges.Action Required
The repository administrator should ensure that the status check named
PR Results(frompr-open.yml) is set as the primary Required Status Check in Branch Protection settings.Thanks for the PR!
Deployments, as required, will be available below:
Please create PRs in draft mode. Mark as ready to enable:
After merge, new images are deployed in: