fix: Trivy action inputs leaking between invocations (#422) (#454)

* fix: use trivy_envs.txt for envs

* test: add test step

* refactor

* refactor

* test

* refactor: use `export` in trivy_envs.txt

* test

* test metadata.json

* test metadata.json

* Clean up envs file better (#422)

- Explicitly rm -f it at start and end of action
- Also remove temporary test steps from action

* Add BATS test for usage of trivy_envs.txt file (#422)

* Add optional step triggered only when Actions Debug logging

Dump the generated environment variables file only when tests are run
with actions debug logging

* Fix to always set env vars into file (#422)

This is done as long as they have a non-empty input value, or a
non-empty default value.

* Clean up env overwriting

- Remove unnecessary debug statements in the action used during testing
- Additional explanatory comments
- Fix to address case where caller explicitly injects environment
  variables, either via env: block on the action call or via GITHUB_ENV

* Further refine env var setting logic (#422)

Noted in documenting this fix that what had been implemented deviated
from the existing configuration priority documentation.  Amended the
implementation of the Action to try and restore that consistency.

* Fix shell syntax error (#422)

---------

Co-authored-by: DmitriyLewen <[email protected]>

Author: rvesse

README.md

@@ -854,6 +854,13 @@ Following inputs can be used as `step.with` keys:
 ### Environment variables
 You can use [Trivy environment variables][trivy-env] to set the necessary options (including flags that are not supported by [Inputs](#inputs), such as `--secret-config`).
 
+**NB** In some older versions of the Action there was a bug that caused inputs from one call to the Action to leak 
+over to subsequent calls to the Action.  This could cause workflows that call the Action multiple times e.g. to run 
+multiple scans, or the same scans with different output formats, to not produce the desired output.  You can see if this
+is the case by looking at the GitHub Actions step information, if the `env` section shown in your Actions output 
+contains `TRIVY_*` environment variables you did not explicitly set then you may be affected by this bug and should 
+upgrade to the latest Action version.
+
 ### Trivy config file
 When using the `trivy-config` [Input](#inputs), you can set options using the [Trivy config file][trivy-config] (including flags that are not supported by [Inputs](#inputs), such as `--secret-config`).
 

action.yaml

@@ -147,21 +147,41 @@ runs:
       env:
         GITHUB_ACTION_PATH: ${{ github.action_path }}
 
+    # Create and Clear Trivy Envs file
+    # See #422 for context
+    - name: Clear Trivy Envs file
+      shell: bash
+      run: |
+        rm -f trivy_envs.txt
+        touch trivy_envs.txt
+
     - name: Set Trivy environment variables
       shell: bash
       run: |
         # Note: There is currently no way to distinguish between undefined variables and empty strings in GitHub Actions.
         # This limitation affects how we handle default values and empty inputs.
         # For more information, see: https://github.com/actions/runner/issues/924
 
-        # Function to set environment variable only if the input is provided and different from default
+        # The following logic implements the configuration priority described in the README:
+        #
+        # Inputs
+        # Environment Variables
+        # Config File
+        # Defaults
+        #
+        # As noted above defaults are awkward to handle as GitHub Actions will inject those values as the input
+        # if the caller doesn't provide them, thus if the input matches the default we don't set it as we
+        # can't tell the difference.  Plus if we did set it when it was the default value then it could potentially 
+        # override an external environment variable, or something in the callers configuration file, which then wouldn't 
+        # match the configuration priority that is documented.
         set_env_var_if_provided() {
           local var_name="$1"
           local input_value="$2"
           local default_value="$3"
         
           if [ ! -z "$input_value" ] && [ "$input_value" != "$default_value" ]; then
-            echo "$var_name=$input_value" >> $GITHUB_ENV
+            # If action was provided with explicit input by the caller set that
+            echo "export $var_name=$input_value" >> trivy_envs.txt
           fi
         }
         
@@ -202,3 +222,11 @@ runs:
 
         # For Trivy
         TRIVY_CACHE_DIR: ${{ inputs.cache-dir }} # Always set
+
+    # Remove Trivy envs to keep envs within this action and avoid env leaks
+    # See #422 for context
+    - name: Remove Trivy Envs file
+      if: ${{ always() }}
+      shell: bash
+      run: |
+        rm -f trivy_envs.txt

entrypoint.sh

@@ -1,6 +1,14 @@
 #!/bin/bash
 set -euo pipefail
 
+# Read TRIVY_* envs from file, previously they were written to the GITHUB_ENV file but GitHub Actions automatically 
+# injects those into subsequent job steps which means inputs from one trivy-action invocation were leaking over to 
+# any subsequent invocation which led to unexpected/undesireable behaviour from a user perspective
+# See #422 for more context around this
+if [ -f ./trivy_envs.txt ]; then
+  source ./trivy_envs.txt
+fi
+
 # Set artifact reference
 scanType="${INPUT_SCAN_TYPE:-image}"
 scanRef="${INPUT_SCAN_REF:-.}"
@@ -54,4 +62,4 @@ if [ "${TRIVY_FORMAT:-}" = "github" ]; then
   fi
 fi
 
-exit $returnCode
+exit $returnCode

test/test.bats

@@ -39,6 +39,7 @@ function reset_envs() {
   for var in $(env | grep '^TRIVY_\|^INPUT_' | cut -d= -f1); do
     unset "$var"
   done
+  rm -f trivy_envs.txt
 }
 
 function compare_files() {
@@ -149,4 +150,31 @@ function compare_files() {
   ./entrypoint.sh
   compare_files tfvars.json ./test/data/with-tf-vars/report.json
   reset_envs
+}
+
+@test "trivy image via environment file" {
+  # trivy image --severity CRITICAL --output image.test knqyf263/vuln-image:1.2.3
+  # Action injects inputs into the script via environment variables
+  echo "export TRIVY_OUTPUT=image.test" >> trivy_envs.txt
+  echo "export TRIVY_SEVERITY=CRITICAL" >> trivy_envs.txt
+  echo "export INPUT_SCAN_TYPE=image" >> trivy_envs.txt
+  echo "export INPUT_SCAN_REF=knqyf263/vuln-image:1.2.3" >> trivy_envs.txt 
+  ./entrypoint.sh
+  compare_files image.test ./test/data/image-scan/report
+  reset_envs
+}
+
+@test "trivy image via environment file overrides env leakages" {
+  # trivy image --severity CRITICAL --output image.test knqyf263/vuln-image:1.2.3
+  # Action injects inputs into the script via environment variables
+  # If caller mixes old and new trivy-action version they could still have env leakage so verify that env vars already
+  # in the env are overridden by those from the envs file
+  export INPUT_SCAN_REF=no/such-image:1.2.3
+  echo "export TRIVY_OUTPUT=image.test" >> trivy_envs.txt
+  echo "export TRIVY_SEVERITY=CRITICAL" >> trivy_envs.txt
+  echo "export INPUT_SCAN_TYPE=image" >> trivy_envs.txt
+  echo "export INPUT_SCAN_REF=knqyf263/vuln-image:1.2.3" >> trivy_envs.txt 
+  ./entrypoint.sh
+  compare_files image.test ./test/data/image-scan/report
+  reset_envs
 }