Harden actions, delete Projects of failed tests, close #831

.github/workflows/github_server.yml

@@ -2,7 +2,7 @@ name: GitHub server installation
 
 on:
   push:
-  # Default opened, reopened, synchronize. "edited" is something else.
+  # Default "opened", "reopened", "synchronize". "edited" fires when the PR title gets edited, etc.
   pull_request_target:
   workflow_dispatch:
     inputs:
@@ -44,7 +44,7 @@ jobs:
         id: github_server
         #### Developer note: you'll need to replace `.` with the path to
         #### the repo and branch. For example:
-        #### suffolkLITLab/ALKiln@v5/action_for_github_server
+        #### suffolkLITLab/ALKiln/action_for_github_server@v5
         uses: ./action_for_github_server
         with:
           CONFIG_CONTENTS: "${{ secrets.CONFIG_CONTENTS }}"

CHANGELOG.md

@@ -45,6 +45,13 @@ Format:
 
 ## [Unreleased]
 
+### Fixed
+- Restored previous behavior - Projects are now deleted even when a workflow fails. See [#831](https://github.com/SuffolkLITLab/ALKiln/issues/831).
+- Fixed typo of comment explaining how to reference the isolated GitHub server action.
+
+### Security
+- Harden security for action input handling. That said, the source of the problem would come from author workflows, so there's a lot more authors can do to help this situation than we can. Workflows triggered by pull requests are a specific place to watch. GitHub already has [some default safeguards](https://docs.github.com/en/actions/managing-workflow-runs/approving-workflow-runs-from-public-forks#about-workflow-runs-from-public-forks) to prevent strangers, specifically first-time contributors, from triggering workflows like that. They can also set their [org](https://docs.github.com/en/organizations/managing-organization-settings/disabling-or-limiting-github-actions-for-your-organization#configuring-required-approval-for-workflows-from-public-forks) or [repo](https://docs.github.com/en/repositories/managing-your-repositorys-settings-and-features/enabling-features-for-your-repository/managing-github-actions-settings-for-a-repository#controlling-changes-from-forks-to-workflows-in-public-repositories) to be more strict about pull requests from outside collaborators.
+
 ### Internal
 - Freeze `docassemblecli` version at 0.0.17. See [#827](https://github.com/SuffolkLITLab/ALKiln/issues/827).
 

action.yml

@@ -22,8 +22,6 @@ inputs:
     description: "Max amount of time to give the server to reload if it needs to"
     required: false
     # no default to let the custom Scenario timeouts create the reload timeout
-  # INSTALL_LOCATION? Will that get confused with the location passed into the command line?
-  # Maybe `INSTALL_LOCATION` could be "playground", "server", or a custom path and we could handle all those.
   INSTALL_METHOD:
     description: "Default is 'playground'. Other option: 'server'. How to install the package onto the server."
     required: false
@@ -32,7 +30,7 @@ inputs:
     description: "The tag expression to use to limit which tests to run. Default is no tag expression."
     required: false
   DOCASSEMBLECLI_VERSION:
-    description: 'Where to get the docassemblecli package. E.g. a GitHub zip or a pypi version.'
+    description: "The version of docassemblecli to install."
     required: false
     default: "0.0.17"
 
@@ -44,19 +42,29 @@ runs:
 
     # Set environment variables
     - name: "ALKiln setup info: Set environment variables"
-      # Human-readable date/time:
-      # https://www.shell-tips.com/linux/how-to-format-date-and-time-in-linux-macos-and-bash/#how-to-format-a-date-in-bash
+      # Safe input handling. Avoid evaluating in bash.
+      env:
+        BRANCH_PATH: ${{ github.ref }}
+        BASE_URL: ${{ inputs.SERVER_URL }}
+        DOCASSEMBLE_DEVELOPER_API_KEY: ${{ inputs.DOCASSEMBLE_DEVELOPER_API_KEY }}
+        ALKILN_VERSION: ${{ inputs.ALKILN_VERSION }}
+        MAX_SECONDS_FOR_SETUP: ${{ inputs.MAX_SECONDS_FOR_SETUP }}
+        SERVER_RELOAD_TIMEOUT_SECONDS: ${{ inputs.SERVER_RELOAD_TIMEOUT_SECONDS }}
+        DOCASSEMBLECLI_VERSION: ${{ inputs.DOCASSEMBLECLI_VERSION }}
       run: |
         # ALKiln setup info: Set environment variables
+        # Human-readable date/time: https://www.shell-tips.com/linux/how-to-format-date-and-time-in-linux-macos-and-bash/#how-to-format-a-date-in-bash
         echo "ARTIFACT_NAME=alkiln-$(date +'%Y-%m-%d at %Hh%Mm%Ss' -u)UTC" >> $GITHUB_ENV
         echo "REPO_URL=${{ github.server_url }}/${{ github.repository }}" >> $GITHUB_ENV
-        echo "BRANCH_PATH=${{ github.ref }}" >> $GITHUB_ENV
-        echo "BASE_URL=${{ inputs.SERVER_URL }}" >> $GITHUB_ENV
-        echo "DOCASSEMBLE_DEVELOPER_API_KEY=${{ inputs.DOCASSEMBLE_DEVELOPER_API_KEY }}" >> $GITHUB_ENV
-        echo "ALKILN_VERSION=${{ inputs.ALKILN_VERSION }}" >> $GITHUB_ENV
-        echo "MAX_SECONDS_FOR_SETUP=${{ inputs.MAX_SECONDS_FOR_SETUP }}" >> $GITHUB_ENV
-        echo "SERVER_RELOAD_TIMEOUT_SECONDS=${{ inputs.SERVER_RELOAD_TIMEOUT_SECONDS }}" >> $GITHUB_ENV
-        echo "DOCASSEMBLECLI_VERSION=${{ inputs.DOCASSEMBLECLI_VERSION }}" >> $GITHUB_ENV
+        echo "BRANCH_PATH=$BRANCH_PATH" >> $GITHUB_ENV
+        # Workflow inputs
+        echo "BASE_URL=$BASE_URL" >> $GITHUB_ENV
+        echo "DOCASSEMBLE_DEVELOPER_API_KEY=$DOCASSEMBLE_DEVELOPER_API_KEY" >> $GITHUB_ENV
+        echo "ALKILN_VERSION=$ALKILN_VERSION" >> $GITHUB_ENV
+        echo "MAX_SECONDS_FOR_SETUP=$MAX_SECONDS_FOR_SETUP" >> $GITHUB_ENV
+        echo "SERVER_RELOAD_TIMEOUT_SECONDS=$SERVER_RELOAD_TIMEOUT_SECONDS" >> $GITHUB_ENV
+        echo "DOCASSEMBLECLI_VERSION=$DOCASSEMBLECLI_VERSION" >> $GITHUB_ENV
+        # Hard-coded internal ALKiln vars
         echo "_ORIGIN=github" >> $GITHUB_ENV
       shell: bash
     - name: "ALKiln setup info: confirm environment variables"
@@ -73,7 +81,7 @@ runs:
     - name: Install ALKiln directly from npm
       run: |
         # ALKiln setup info: Install ALKiln directly from npm
-        npm install -g @suffolklitlab/alkiln@$ALKILN_VERSION
+        npm install -g "@suffolklitlab/alkiln@$ALKILN_VERSION"
       shell: bash
 
     # Install on playground
@@ -87,7 +95,7 @@ runs:
       if: ${{ inputs.INSTALL_METHOD == 'playground' }}
       run: |
         # ALKiln setup info: Create a Project and install the package from GitHub
-        pip install docassemblecli==$DOCASSEMBLECLI_VERSION
+        pip install "docassemblecli==$DOCASSEMBLECLI_VERSION"
         alkiln-setup
       shell: bash
     - name: "ALKiln setup ERROR"
@@ -108,12 +116,14 @@ runs:
     # run tests
     - name: "ALKiln info: Run tests"
       if: ${{ success() }}
-      run: alkiln-run ${{ inputs.ALKILN_TAG_EXPRESSION || github.event.inputs.tags && format('{0}', github.event.inputs.tags) }}
+      env:
+        ALKILN_TAG_EXPRESSION: ${{ inputs.ALKILN_TAG_EXPRESSION || github.event.inputs.tags && format('{0}', github.event.inputs.tags) }}
+      run: alkiln-run $ALKILN_TAG_EXPRESSION
       shell: bash
 
     # on playground, delete project
     - name: "ALKiln info: Try to delete the project from the playground of the docassemble test account."
-      if: ${{ inputs.INSTALL_METHOD == 'playground' }}
+      if: ${{ always() && inputs.INSTALL_METHOD == 'playground' }}
       run: alkiln-takedown
       shell: bash
 

action_for_github_server/action.yml

@@ -24,7 +24,7 @@ inputs:
     required: false
     default: 600  # 10 min
   DOCASSEMBLECLI_VERSION:
-    description: 'Where to get the docassemblecli package. E.g. a GitHub zip or a pypi version.'
+    description: "The version of docassemblecli to install."
     required: false
     default: "0.0.17"
 
@@ -48,13 +48,17 @@ runs:
       - uses: actions/checkout@v3
 
       - name: "ALKiln GitHub server - Set environment variables"
+        env:
+          MAX_SECONDS_FOR_DOCKER: ${{ inputs.MAX_SECONDS_FOR_DOCKER }}
+          DOCASSEMBLECLI_VERSION: ${{ inputs.DOCASSEMBLECLI_VERSION }}
         run: |
+          # ALKiln GitHub server info: Set environment variables
+          echo "MAX_SECONDS_FOR_DOCKER=$MAX_SECONDS_FOR_DOCKER" >> $GITHUB_ENV
+          echo "DOCASSEMBLECLI_VERSION=$DOCASSEMBLECLI_VERSION" >> $GITHUB_ENV
           echo "DA_ADMIN_API_KEY=abcd1234abcd1234abcd5678abdc5678" >> $GITHUB_ENV
-          echo "MAX_SECONDS_FOR_DOCKER=${{ inputs.MAX_SECONDS_FOR_DOCKER }}" >> $GITHUB_ENV
           echo "DA_ADMIN_EMAIL=admin@example.com" >> $GITHUB_ENV
-          # Login will fail if this is "password"
+          # Login will fail if this value is "password"
           echo "DA_ADMIN_PASSWORD=@123abcdefg" >> $GITHUB_ENV
-          echo "DOCASSEMBLECLI_VERSION=${{ inputs.DOCASSEMBLECLI_VERSION }}" >> $GITHUB_ENV
         shell: bash
       - id: api_key_output_step
         run: |
@@ -64,35 +68,37 @@ runs:
         shell: bash
 
       - name: "ALKiln GitHub server - Get config"
+        env:
+          CONFIG: ${{ inputs.CONFIG_CONTENTS }}
         run: |
           # ALKiln GitHub server info: config values
           mkdir /tmp/config
           # Worth making this silent by defining a variable with > /dev/null...?
-          if [ -n "${{ inputs.CONFIG_CONTENTS }}" ]
+          if [ -n "$CONFIG" ]
           then
             echo "ALKiln GitHub server - The developer did provide a custom docassemble config file as an input into this action."
-            echo "${{ inputs.CONFIG_CONTENTS }}" > /tmp/config/myconfig.yml
+            echo "$CONFIG" > /tmp/config/myconfig.yml
             echo "CONFIG_ARGS=--env DA_CONFIG=/tmp/config/myconfig.yml --volume /tmp/config:/tmp/config " >> $GITHUB_ENV
           else
             echo "ALKiln GitHub server - The developer did NOT provide a custom docassemble config file as an input into this action. Docassemble will use its default config."
             echo "CONFIG_ARGS=" >> $GITHUB_ENV
           fi
         shell: bash
 
-      # GitHub masks secret values like the config, so we don't really
-      # need to stop that yet, but users may start using variables, so
-      # a question: If in future we allow variables instead of just secrets,
+      # Question: If in future we allow GitHub variables instead of just secrets,
       # should we let those be visible or prevent them along with the docker logs?
-
-      # Also, does this leave GitHub job environment output While just
-      # hiding the output from the job console?
+      # Also, does GitHub hide job environment output of secrets as well as
+      # hiding the secrets' output from the job console?
 
       # Define the var to prevent/show docker output
+      # TODO: Why are we only doing this for manually triggered workflows?
       - name: Env var for showing docker output
+        env:
+          SHOW_DOCKER_OUTPUT: ${{ inputs.SHOW_DOCKER_OUTPUT }}
         run: |
           # ALKiln GitHub server info: Docker logs visibility decision
           if [[ "${{ github.event_name }}" == "workflow_dispatch" ]]; then
-            echo "OUTPUT_DOCKER=${{ inputs.SHOW_DOCKER_OUTPUT }}" >> $GITHUB_ENV
+            echo "OUTPUT_DOCKER=$SHOW_DOCKER_OUTPUT" >> $GITHUB_ENV
           else
             echo "OUTPUT_DOCKER=false" >> $GITHUB_ENV
           fi
@@ -110,31 +116,36 @@ runs:
           printf '===\nALKiln GitHub server - GitHub will now show the output of docker creation. ALKiln cannot control this output. If you want to prevent output, use "with:" and set the input "SHOW_DOCKER_OUTPUT" to "false". This output will only be visible to those who can already see the action output, like people with admin or write permissions.\n==='
         shell: bash
 
+      # TODO: Reduce these to 1 block by putting ` > /dev/null 2>&1` into an env var
       # No output
       - name: "ALKiln GitHub server - Download and start docker silently"
         if: ${{ env.OUTPUT_DOCKER == 'false' }}
+        env:
+          CONFIG_ARGS: ${{ env.CONFIG_ARGS }}
         run: |
           # ALKiln GitHub server info: Silent docker pull/run
           docker pull jhpyle/docassemble > /dev/null 2>&1
           docker run --name docassemble_container \
-                     --env DA_ADMIN_EMAIL="${{ env.DA_ADMIN_EMAIL }}" \
-                     --env DA_ADMIN_PASSWORD="${{ env.DA_ADMIN_PASSWORD }}" \
-                     --env DA_ADMIN_API_KEY="${{ env.DA_ADMIN_API_KEY }}" \
-                     ${{ env.CONFIG_ARGS }} \
+                     --env DA_ADMIN_EMAIL="$DA_ADMIN_EMAIL" \
+                     --env DA_ADMIN_PASSWORD="$DA_ADMIN_PASSWORD" \
+                     --env DA_ADMIN_API_KEY="$DA_ADMIN_API_KEY" \
+                     $CONFIG_ARGS \
                      --cap-add SYS_PTRACE \
                      --memory="4gb" -d -p 80:80 jhpyle/docassemble > /dev/null 2>&1
         shell: bash
       # With output
       - name: "ALKiln GitHub server - Download and start docker"
         if: ${{ env.OUTPUT_DOCKER == 'true' }}
+        env:
+          CONFIG_ARGS: ${{ env.CONFIG_ARGS }}
         run: |
           # ALKiln GitHub server info: Docker pull/run
           docker pull jhpyle/docassemble
           docker run --name docassemble_container \
-                     --env DA_ADMIN_EMAIL="${{ env.DA_ADMIN_EMAIL }}" \
-                     --env DA_ADMIN_PASSWORD="${{ env.DA_ADMIN_PASSWORD }}" \
-                     --env DA_ADMIN_API_KEY="${{ env.DA_ADMIN_API_KEY }}" \
-                     ${{ env.CONFIG_ARGS }} \
+                     --env DA_ADMIN_EMAIL="$DA_ADMIN_EMAIL" \
+                     --env DA_ADMIN_PASSWORD="$DA_ADMIN_PASSWORD" \
+                     --env DA_ADMIN_API_KEY="$DA_ADMIN_API_KEY" \
+                     $CONFIG_ARGS \
                      --cap-add SYS_PTRACE \
                      --memory="4gb" -d -p 80:80 jhpyle/docassemble
         shell: bash
@@ -193,7 +204,7 @@ runs:
         with:
           python-version: '3.10'
       # No output
-      - run: pip install docassemblecli==$DOCASSEMBLECLI_VERSION > /dev/null 2>&1
+      - run: pip install "docassemblecli==$DOCASSEMBLECLI_VERSION" > /dev/null 2>&1
         if: ${{ env.OUTPUT_DOCKER == 'false' }}
         shell: bash
       # Install the package, the current directory (.), onto the docassemble server
@@ -202,7 +213,7 @@ runs:
         shell: bash
 
       # With output
-      - run: pip install docassemblecli==$DOCASSEMBLECLI_VERSION
+      - run: pip install "docassemblecli==$DOCASSEMBLECLI_VERSION"
         if: ${{ env.OUTPUT_DOCKER == 'true' }}
         shell: bash
       # Install the package, the current directory (.), onto the docassemble server