Provide an output that can be used in later steps to avoid duplicate runs

erzz commented 1 week ago

The request

As another step towards reducing load on the rate limiting - could you provide an action output that we could use like result=fail. This would be decoupled completely from exit code (for which the behavior would remain the same as it is today, depending on the exit-code flag)

The background

To explain where this would be useful - our typical flow up to now has been to run Trivy twice in a workflow

once to produce the nice readable table view (with exit-code 1) so engineers can see the feedback direct in the workflow if it should fail a scan and we can "break the build"
then immediately again (if: always())to get in SARIF format for upload to GHAS

What would an output solve?

Mainly that we can just start to use convert instead of a second run whilst presenting a workflow output that is easy and obvious to an engineer to see their results in a workflow when it fails.

To do this now in a less obvious format for the consumer of the workflow I have something like:

- name: Trivy Image Scan
  id: scan
  uses: aquasecurity/trivy-action@0.27.0
  with:
      image-ref: some-image
      exit-code: 1
      format: json
      output: "report.json"

- name: Setup trivy for SARIF
  if: always()
  uses: aquasecurity/setup-trivy@v0.2.0

- name: Convert to SARIF
  if: always()
  run: |
      trivy convert --format=sarif --output="results.sarif" "results.json"

- name: Upload results to GitHub Security
  if: always()
  uses: github/codeql-action/upload-sarif@v3
  with:
      sarif_file: "results.sarif"

- name: View readable report
  if: steps.scan.outcome == 'failure'
  run: |
      trivy convert --format=table results.json && exit 1

The problems with this are:

there will be two failed steps. One of which is the original scan step with no output because its written to file (confusing to engineer and they may assume it was a runtime error) and then a second step further down the job log to find the actual results in a readable format.
I cannot trigger the readable report without also failing the scan step
Its all a lot of faff :)

What would the solution look like

- name: Trivy Image Scan
  id: scan
  uses: aquasecurity/trivy-action@0.27.0
  with:
      image-ref: some-image
      exit-code: 0                                               <<<------ engineer will not look here
      format: json
      output: "report.json"

- name: Setup trivy for SARIF
  if: always()
  uses: aquasecurity/setup-trivy@v0.2.0

- name: Convert to SARIF
  if: always()
  run: |
      trivy convert --format=sarif --output="results.sarif" "results.json"

- name: Upload results to GitHub Security
  if: always()
  uses: github/codeql-action/upload-sarif@v3
  with:
      sarif_file: results.sarif

- name: View results
  if: steps.scan.outputs.result == 'fail'
  run: |
      trivy convert --format=table "results.json && exit 1       <<<----- nice obvious end of job view if there are failures

So now the workflow output makes total sense to a consumer when there are findings, you get less requests for the DB because we arent running it twice.

Alternative suggestions

Adding the convert command to the action which would also be nice - but I suspect much more work than a simple addition of an output?

Allow multiple outputs in a single execution - but you guys already rejected that many times and is the reason you added the excellent convert command :)

Add a upload-sarif flag and you handle the creation of and upload of the sarif directly in the initial execution - also a lot more work but would be nice

You could go bananas with outputs like total-findings, critical-findings, high-findings etc etc

AshishBhadouria commented 1 week ago

As a consumer of the trivy findings, I endorse this request.

simar7 commented 1 week ago

Maybe I'm misunderstanding something here but running trivy twice in the same pipeline job doesn't request a DB twice, does it?

erzz commented 1 week ago

@simar7 It shouldn't (though it has in recentish runs I think because of now-resolved bug)

I try to be a good citizen and have 2 steps in the same job - I have seen many having parallel jobs (for speed).

But pushing convert as the correct method to approach multiple and/or dependent/conditional formats requires either the output(s), or convert, to be better exposed by the action. I just proposed the lowest hanging fruit to achieve it as the main suggestion :)

I am pretty close to throwing out the action anyway now given we have the new setup-trivy action.... its a lot of ugly flag management in yaml-wrapped shell to just use native trivy... but most of my suggestions those suggestions would alleviate the need for that

aquasecurity / trivy-action