ci: run code scan only when dependency files change - #219
yaroslavmokflmg wants to merge 7 commits into
Conversation
🦩 Flamingo Code Review2 finding(s) — 0 action required · 2 recommended · 0 informational Mode: advisory · 2 defect(s) outside any rule Inline comments: 2 new Change set
Need another pass? Commits pushed after this review are not reviewed automatically.
Prefer typing? Comment React 👍/👎 on inline comments to teach the reviewer. Started 2026-09-28 08:26 UTC · updated 2026-09-28 08:29 UTC · workflow run |
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
| - name: Scan code | ||
| if: steps.should_run.outputs.deps == 'true' | ||
| uses: ./.github/steps/trivy | ||
| with: | ||
| scan: code |
There was a problem hiding this comment.
🦩 🟠 [warn/recommended] Trivy code scan now silently skipped for PRs, undermining required security gate
The 'Scan code' step (Trivy vulnerability scan) is now gated behind steps.should_run.outputs.deps == 'true', meaning the mandatory code security scan only runs when files matching the deps filter (pom.xml, package.json, go.mod, Dockerfile*, etc.) changed. Any PR that changes application source code without touching those specific manifest/lock files (e.g. changes to .go source files that introduce a vulnerable pattern, or changes elsewhere not covered by the filter globs) will skip the scan entirely, even though the job is still reported as passing (skipped steps do not fail the job). This weakens the security posture of a workflow whose stated purpose is code scanning on every pull request, and should be paired with clear reasoning/documentation on why skipping is safe, or the filter should be broadened to include source file changes relevant to the scan.
Evidence
- name: Scan code
if: steps.should_run.outputs.deps == 'true'
uses: ./.github/steps/trivy
with:
scan: code
🤖 Prompt for AI agents
In .github/workflows/test.yml around lines 74-78, address this code-review finding: Trivy code scan now silently skipped for PRs, undermining required security gate.
The 'Scan code' step (Trivy vulnerability scan) is now gated behind steps.should_run.outputs.deps == 'true', meaning the mandatory code security scan only runs when files matching the deps filter (pom.xml, package.json, go.mod, Dockerfile*, etc.) changed. Any PR that changes application source code without touching those specific manifest/lock files (e.g. changes to .go source files that introduce a vulnerable pattern, or changes elsewhere not covered by the filter globs) will skip the scan entirely, even though the job is still reported as passing (skipped steps do not fail the job). This weakens the security posture of a workflow whose stated purpose is code scanning on every pull request, and should be paired with clear reasoning/documentation on why skipping is safe, or the filter should be broadened to include source file changes relevant to the scan.
The flagged code:
```
- name: Scan code
if: steps.should_run.outputs.deps == 'true'
uses: ./.github/steps/trivy
with:
scan: code
```
Make the minimal change that resolves the finding; do not refactor unrelated code.
confidence: 55 — react 👍/👎 to teach the reviewer
| - name: Check if scan should run | ||
| id: should_run | ||
| uses: dorny/paths-filter@v4.0.2 | ||
| with: | ||
| filters: | | ||
| deps: | ||
| - '**/pom.xml' | ||
| - '.mvn/**' | ||
| - '**/package.json' | ||
| - '**/package-lock.json' | ||
| - '**/yarn.lock' | ||
| - '**/pnpm-lock.yaml' | ||
| - '**/go.mod' | ||
| - '**/go.sum' | ||
| - '**/Cargo.toml' | ||
| - '**/Cargo.lock' | ||
| - '**/Dockerfile*' | ||
| - '.trivyignore' | ||
| - '.github/steps/trivy/**' |
There was a problem hiding this comment.
🦩 🟠 [warn/recommended] paths-filter runs against the base checkout, not the PR head, likely misdetecting changed files
The new 'Check if scan should run' step using dorny/paths-filter runs BEFORE the 'Checkout' step that fetches the PR head SHA (actions/checkout with ref: ${{ github.event.pull_request.head.sha }}). Since paths-filter is not itself preceded by a checkout of the PR head in this job, it evaluates against whatever ref the runner's default checkout provides (or none at all, since the explicit Checkout step is now itself gated by the filter's own output). This can produce incorrect deps filtering: on the initial run there is no checkout yet, so paths-filter falls back to comparing commit refs via the GitHub API only if a 'base'/'ref' is set, but no explicit base/head input is provided here, making the result depend on default behavior rather than the intended PR diff. Combined with the fact that the Checkout step is now conditioned on the filter's own output, if the filter step misbehaves (e.g., due to missing checkout context) the whole job silently skips the security scan for PRs that do touch dependency files, which is a correctness/security regression for a job whose entire purpose is dependency vulnerability scanning.
Evidence
- name: Check if scan should run
id: should_run
uses: dorny/paths-filter@v4.0.2
with:
filters: |
deps:
- '**/pom.xml'
- '.mvn/**'
- '**/package.json'
- '**/package-lock.json'
- '**/yarn.lock'
- '**/pnpm-lock.yaml'
🤖 Prompt for AI agents
In .github/workflows/test.yml around lines 47-65, address this code-review finding: paths-filter runs against the base checkout, not the PR head, likely misdetecting changed files.
The new 'Check if scan should run' step using dorny/paths-filter runs BEFORE the 'Checkout' step that fetches the PR head SHA (actions/checkout with ref: ${{ github.event.pull_request.head.sha }}). Since paths-filter is not itself preceded by a checkout of the PR head in this job, it evaluates against whatever ref the runner's default checkout provides (or none at all, since the explicit Checkout step is now itself gated by the filter's own output). This can produce incorrect deps filtering: on the initial run there is no checkout yet, so paths-filter falls back to comparing commit refs via the GitHub API only if a 'base'/'ref' is set, but no explicit base/head input is provided here, making the result depend on default behavior rather than the intended PR diff. Combined with the fact that the Checkout step is now conditioned on the filter's own output, if the filter step misbehaves (e.g., due to missing checkout context) the whole job silently skips the security scan for PRs that do touch dependency files, which is a correctness/security regression for a job whose entire purpose is dependency vulnerability scanning.
The flagged code:
```
- name: Check if scan should run
id: should_run
uses: dorny/paths-filter@v4.0.2
with:
filters: |
deps:
- '**/pom.xml'
- '.mvn/**'
- '**/package.json'
- '**/package-lock.json'
- '**/yarn.lock'
- '**/pnpm-lock.yaml'
- '**/go.mod'
- '**/go.sum'
- '**/Cargo.toml'
- '**/Cargo.lock'
- '**/Dockerfile*'
- '.trivyignore'
- '.github/steps/trivy/**'
```
Make the minimal change that resolves the finding; do not refactor unrelated code.
confidence: 45 — react 👍/👎 to teach the reviewer
yaroslavmokflmg
changed the title
yaroslavmokflmg
changed the title
This branch has not been deployed
1 participant
The trivy action now checks (dorny/paths-filter) whether the PR touched anything under the scan path, minus skip-dirs, or the root pom.xml, .trivyignore or the action itself; otherwise the scan is skipped. Same action in every repo, workflows only grant pull-requests: read.
Change-Set: hotfix-conditional-code-scan
Change set
flamingo-stack/fleetmdm#219: these pull requests are one change, reviewed together.Merge order: #219 → flamingo-stack/meshcentral#213 → flamingo-stack/openframe-oss-frontend#498 → flamingo-stack/openframe-oss-tenant#2379
Linked by the
Depends-On/Change-Setlines in these descriptions; this block is maintained by the hub.