[DEVEX-111] Secure workflows from external inputs (#9)

pagopa · Apr 29, 2024 · eb49ec5 · eb49ec5
1 parent 5c70c46
commit eb49ec5
Show file tree

Hide file tree

Showing 2 changed files with 46 additions and 26 deletions.
diff --git a/.github/workflows/infra_apply.yaml b/.github/workflows/infra_apply.yaml
@@ -55,27 +55,36 @@ jobs:
       # The directory the value is then available in ${{ steps.directory.outputs.dir }}
       - name: Set directory
         id: directory
+        env:
+          ENVIRONMENT: ${{ inputs.environment }}
+          REGION: ${{ inputs.region }}
+          BASE_PATH: ${{ inputs.base_path }}
         run: |
-          set -e
-          if [ -z "${{ inputs.environment }}" ] || [ -z "${{ inputs.region }}" ]; then
+          set -euo pipefail
+
+          if [ -z "$ENVIRONMENT" ] || [ -z "$REGION" ]; then
             echo "Both environment and region must be provided."
             exit 1
           else
             # The directory is expected to be in the format
-            #  ${inputs.base_path}/${{ inputs.environment }}/${{ inputs.region }}
+            #  ${inputs.base_path}/"$ENVIRONMENT"/"$REGION"
             # Example: infra/resources/prod/westeurope
-            echo "dir=${{ inputs.base_path}}/${{ inputs.environment }}/${{ inputs.region }}" >> $GITHUB_OUTPUT
+            printf "dir=%q/%q/%q" "$BASE_PATH" "$ENVIRONMENT" "$REGION" >> "$GITHUB_OUTPUT"
           fi
 
       - uses: actions/checkout@b4ffde65f46336ab88eb53be808477a3936bae11 # v4.1.1
         name: Checkout
 
       - name: Set Environment Variables
         if: ${{ inputs.env_vars }}
+        env:
+          ENV_VARS: ${{ inputs.env_vars }}
         run: |
-          for i in "${{ inputs.env_vars }}"
+          set -euo pipefail
+
+          for i in "$ENV_VARS[@]"
           do
-            printf "%s\n" "$i" >> $GITHUB_ENV
+            printf "%q\n" "$i" >> "$GITHUB_ENV"
           done
 
       - name: Azure Login
@@ -88,7 +97,9 @@ jobs:
       - name: Set Terraform Version
         id: set-terraform-version
         run: |
-          echo "terraform_version=$(cat .terraform-version)" >> $GITHUB_OUTPUT
+          set -eu
+          terraform_version=$(cat .terraform-version)
+          echo "terraform_version=$terraform_version" >> "$GITHUB_OUTPUT"
 
       - uses: hashicorp/setup-terraform@a1502cd9e758c50496cc9ac5308c4843bcd56d36 # v3.0.0
         name: Setup Terraform

diff --git a/.github/workflows/infra_plan.yaml b/.github/workflows/infra_plan.yaml
@@ -52,27 +52,36 @@ jobs:
       # The directory the value is then available in ${{ steps.directory.outputs.dir }}
       - name: Set directory
         id: directory
+        env:
+          ENVIRONMENT: ${{ inputs.environment }}
+          REGION: ${{ inputs.region }}
+          BASE_PATH: ${{ inputs.base_path }}
         run: |
-          set -e
-          if [ -z "${{ inputs.environment }}" ] || [ -z "${{ inputs.region }}" ]; then
+          set -euo pipefail
+
+          if [ -z "$ENVIRONMENT" ] || [ -z "$REGION" ]; then
             echo "Both environment and region must be provided."
             exit 1
           else
             # The directory is expected to be in the format
             #  infra/resources/${{ inputs.environment }}/${{ inputs.region }}
             # Example: infra/resources/prod/westeurope
-            echo "dir=${{ inputs.base_path}}/${{ inputs.environment }}/${{ inputs.region }}" >> $GITHUB_OUTPUT
+            printf "dir=%q/%q/%q" "$BASE_PATH" "$ENVIRONMENT" "$REGION" >> "$GITHUB_OUTPUT"
           fi
 
       - uses: actions/checkout@b4ffde65f46336ab88eb53be808477a3936bae11 # v4.1.1
         name: Checkout
 
       - name: Set Environment Variables
         if: ${{ inputs.env_vars }}
+        env:
+          ENV_VARS: ${{ inputs.env_vars }}
         run: |
-          for i in "${{ inputs.env_vars }}"
+          set -euo pipefail
+
+          for i in "$ENV_VARS[@]"
           do
-            printf "%s\n" $i >> $GITHUB_ENV
+            printf "%q\n" "$i" >> "$GITHUB_ENV"
           done
 
       - name: Azure Login
@@ -85,7 +94,9 @@ jobs:
       - name: Set Terraform Version
         id: set-terraform-version
         run: |
-          echo "terraform_version=$(cat .terraform-version)" >> $GITHUB_OUTPUT
+          set -eu
+          terraform_version=$(cat .terraform-version)
+          printf "terraform_version=$terraform_version" >> "$GITHUB_OUTPUT"
 
       - uses: hashicorp/setup-terraform@a1502cd9e758c50496cc9ac5308c4843bcd56d36 # v3.0.0
         name: Setup Terraform
@@ -105,26 +116,27 @@ jobs:
         id: plan
         working-directory: ${{ steps.directory.outputs.dir }}
         run: |
-          set -e
+          terraform plan -lock-timeout=3000s -no-color 2>&1 | tee plan_output.txt
 
-          echo "no output" > plan_output_multiline.txt
-          terraform plan -lock-timeout=3000s -no-color | tee plan_output.txt
           OUTPUT=$(grep -Ev "Refreshing state|state lock|Reading|Read" plan_output.txt | tail -c 60000)
-          echo "$OUTPUT" > plan_output_multiline.txt
+          printf "$OUTPUT" > plan_output_multiline.txt
 
-          if [ $? -ne 0 ]; then
-            echo "result=failed" >> $GITHUB_OUTPUT
+          if grep -q "::error::Terraform exited with code" plan_output.txt; then
+            echo "failed"
+            exit 1
           fi
 
       # Post the plan output in the PR
       # The plan output is posted in a comment in the PR
       - uses: actions/github-script@60a0d83039c74a4aee543508d2ffcb1c3799cdea # v7.0.1
         name: Post Plan on PR
-        if: success() && github.event_name == 'pull_request'
+        id: comment
+        if: always() && github.event_name == 'pull_request'
         with:
           script: |
             const fs = require('fs');
             const output = fs.readFileSync('${{ steps.directory.outputs.dir }}/plan_output_multiline.txt', 'utf8');
+            const status = '${{ steps.plan.outcome }}'
             const { data: comments } = await github.rest.issues.listComments({
               owner: context.repo.owner,
               repo: context.repo.repo,
@@ -133,7 +145,7 @@ jobs:
             const botComment = comments.find(comment => {
               return comment.user.type === 'Bot' && comment.body.includes('Terraform Plan')
             })
-            const commentBody = `#### Terraform Plan ('${{ steps.directory.outputs.dir }}') 📖
+            const commentBody = `#### 📖 Terraform Plan ('${{ steps.directory.outputs.dir }}') - ${status}
             <details>
             <summary>Terraform Plan</summary>
 
@@ -161,9 +173,6 @@ jobs:
 
       # Fail the workflow if the Terraform plan failed
       - name: Check Terraform Plan Result
-        if: always()
+        if: always() && steps.plan.outcome != 'success'
         run: |
-          if [ "${{ steps.plan.outputs.result }}" == "failed" ]; then
-            echo "::error::Terraform plan failed"
-            exit 1
-          fi
+          exit 1