Skip to content

github: isolate trivy scan from credentials and SARIF upload#11596

Open
edsiper wants to merge 1 commit intomasterfrom
travis-check
Open

github: isolate trivy scan from credentials and SARIF upload#11596
edsiper wants to merge 1 commit intomasterfrom
travis-check

Conversation

@edsiper
Copy link
Member

@edsiper edsiper commented Mar 20, 2026


Fluent Bit is licensed under Apache 2.0, by submitting this pull request I understand that this code will be released under the terms of that license.

Summary by CodeRabbit

  • Chores
    • Reorganized security scanning workflow job structure for improved isolation
    • Trivy vulnerability scanning results now automatically uploaded to GitHub Security tab

Signed-off-by: Eduardo Silva <eduardo@chronosphere.io>
@coderabbitai
Copy link

coderabbitai bot commented Mar 20, 2026

📝 Walkthrough

Walkthrough

The Trivy scanning workflow was restructured to isolate permissions and responsibilities. A new prepare-images job handles registry login and image preparation, uploading per-architecture artifacts. The trivy-latest job now loads pre-prepared images without registry credentials. A new upload-trivy-results job uploads SARIF findings to GitHub Security with minimal permissions.

Changes

Cohort / File(s) Summary
Trivy Workflow Restructuring
.github/workflows/cron-trivy.yaml
Reorganized scanning workflow into three jobs: prepare-images (registry login, image pulling, artifact export), trivy-latest (image loading and vulnerability scanning with permissions: {}), and upload-trivy-results (SARIF upload to GitHub Security). Removed secondary local output step and changed artifact handling.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Possibly related PRs

Suggested labels

ci, security

Suggested reviewers

  • niedbalski
  • patrick-stephens
  • celalettin1286

Poem

🐰 A workflow once bundled so tight,
Now splits into jobs, each just right!
Images prepared, Trivy scans safe,
No credentials spilled—clean slate!
Security strengthened, hop-hop-hooray! ✨

🚥 Pre-merge checks | ✅ 3
✅ Passed checks (3 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title accurately describes the main change: isolating Trivy scanning from registry credentials and SARIF upload functionality.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch travis-check

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Tip

CodeRabbit can generate a title for your PR based on the changes with custom instructions.

Set the reviews.auto_title_instructions setting to generate a title for your PR based on the changes in the PR with custom instructions.

Copy link

@chatgpt-codex-connector chatgpt-codex-connector bot left a comment

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: e062fef36f

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".


# Run Trivy with no registry credentials and no GitHub write permissions.
trivy-latest:
needs: prepare-images

Choose a reason for hiding this comment

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

P1 Badge Avoid gating all scans on the full fetch matrix

Because trivy-latest now needs: prepare-images, GitHub will wait for the entire fetch matrix to succeed before starting any scan leg. Since prepare-images no longer has continue-on-error: true, a single failure while pulling/exporting one architecture (for example, a missing linux/arm/v7 image or a transient registry/artifact error) causes all three scan jobs and all SARIF uploads to be skipped. In the previous workflow only the failing architecture was lost, so this change turns a partial outage into a complete loss of security-scan coverage.

Useful? React with 👍 / 👎.

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In @.github/workflows/cron-trivy.yaml:
- Around line 67-72: Remove the job-level "continue-on-error: true" from the
trivy-latest matrix job (the job named "trivy-latest") so scan/artifact/SARIF
upload failures are not masked; keep advisory behavior by configuring Trivy's
own exit-code settings in the Trivy step instead, and rely on existing
orchestration controls (fail-fast: false on the matrix and downstream steps
using if: ${{ always() }}) to prevent the whole workflow from failing while
still surfacing real job-level errors.

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: f138ffca-0e93-4c2a-b0cc-16619e8f6399

📥 Commits

Reviewing files that changed from the base of the PR and between 6ab5965 and e062fef.

📒 Files selected for processing (1)
  • .github/workflows/cron-trivy.yaml

Comment on lines +67 to +72
trivy-latest:
needs: prepare-images
name: ${{ matrix.arch }} container scan
runs-on: [ ubuntu-latest ]
continue-on-error: true
permissions: {}
Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

🧩 Analysis chain

🌐 Web query:

In GitHub Actions, what does job-level continue-on-error: truedo for failed steps and matrix jobs, and what is the defaultexit-codebehavior ofaquasecurity/trivy-actionwhenexit-code is omitted?

💡 Result:

In GitHub Actions, setting continue-on-error: true at the job level allows subsequent jobs that depend on it (via needs) to run even if the job encounters a failure in one of its steps. However, within the job itself, execution stops at the first failed step—subsequent steps in the same job do not run. Failed steps are still marked as failed (with steps..outcome and steps..conclusion set to 'failure'), but the job's overall result is treated as 'success' for dependency purposes (e.g., needs..result is 'success'). The workflow status remains failed, and the job shows a red X in the UI indicating internal failure. For matrix jobs (using strategy.matrix), continue-on-error: true at the job level applies to the entire matrix strategy. If any matrix leg fails, the job continues (subsequent jobs run), but unlike non-matrix jobs with strategy.fail-fast: false, the workflow may still succeed overall unless configured otherwise. Individual failed matrix legs are marked failed, but the job result is 'success' for dependencies. When exit-code is omitted in aquasecurity/trivy-action, the default is 0. Trivy exits with code 0 by default even if vulnerabilities are found (unless overridden via exit-code: '1' or config). This means the step succeeds regardless of findings, allowing workflows to continue and upload results (e.g., SARIF for code scanning).

Citations:


🏁 Script executed:

cat -n .github/workflows/cron-trivy.yaml | sed -n '60,130p'

Repository: fluent/fluent-bit

Length of output: 3232


Don't mask scan and SARIF upload failures with job-level continue-on-error.

If you want findings to stay advisory, keep that policy in Trivy's exit-code setting, not on the whole job. At the job level, this mainly swallows real breakage—artifact download/load failures, DB fetch issues, or SARIF upload failures—so coverage can be lost without the workflow failing. The orchestration you need (fan-out/fan-in across matrix jobs) is already handled by fail-fast: false and if: ${{ always() }}.

Suggested changes
  trivy-latest:
    needs: prepare-images
    name: ${{ matrix.arch }} container scan
    runs-on: [ ubuntu-latest ]
-   continue-on-error: true
    permissions: {}
    strategy:
      fail-fast: false
  upload-trivy-results:
    needs: trivy-latest
    name: ${{ matrix.arch }} SARIF upload
    runs-on: [ ubuntu-latest ]
    if: ${{ always() }}
-   continue-on-error: true
    permissions:
      contents: read
      security-events: write
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
trivy-latest:
needs: prepare-images
name: ${{ matrix.arch }} container scan
runs-on: [ ubuntu-latest ]
continue-on-error: true
permissions: {}
trivy-latest:
needs: prepare-images
name: ${{ matrix.arch }} container scan
runs-on: [ ubuntu-latest ]
permissions: {}
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In @.github/workflows/cron-trivy.yaml around lines 67 - 72, Remove the job-level
"continue-on-error: true" from the trivy-latest matrix job (the job named
"trivy-latest") so scan/artifact/SARIF upload failures are not masked; keep
advisory behavior by configuring Trivy's own exit-code settings in the Trivy
step instead, and rely on existing orchestration controls (fail-fast: false on
the matrix and downstream steps using if: ${{ always() }}) to prevent the whole
workflow from failing while still surfacing real job-level errors.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant