Merge pull request #11 from 1Password/eddy/fix-injection

Improve the way input is processed to avoid command injection

.github/workflows/test.yml

@@ -15,7 +15,6 @@ jobs:
       - name: Configure 1Password Connect
         uses: ./configure # 1password/load-secrets-action/configure@<version>
         with:
-          use-connect: yes
           connect-host: http://localhost:8080
           connect-token: ${{ secrets.OP_CONNECT_TOKEN }}
       - name: Load secrets
@@ -50,46 +49,3 @@ jobs:
         run: printenv
       - name: Assert test secret values again
         run: ./tests/assert-env-set.sh
-  test-user:  
-    runs-on: ubuntu-latest
-    steps:
-      - uses: actions/checkout@v2
-      - name: Configure 1Password with a user
-        uses: ./configure # 1password/load-secrets-action/configure@<version>
-        with:
-          user-domain: ${{ secrets.USER_DOMAIN }}
-          user-email: ${{ secrets.USER_EMAIL }}
-          user-key: ${{ secrets.USER_KEY }}
-          user-pwd: ${{ secrets.USER_PWD }}
-      - name: Load secrets
-        uses: ./ # 1password/load-secrets-action@<version>
-        env:
-          SECRET: op://p4a7xwirm3lcwtu6okulbhym5i/mybhbbwhewncjb67cy4v7hjpgi/password
-          SECRET_IN_SECTION: op://p4a7xwirm3lcwtu6okulbhym5i/mybhbbwhewncjb67cy4v7hjpgi/test-section/password
-          UNMASKED_VALUE: op://p4a7xwirm3lcwtu6okulbhym5i/mybhbbwhewncjb67cy4v7hjpgi/test-section/username
-      - name: Load multiline secret
-        uses: ./ # 1password/load-secrets-action@<version>
-        env:
-          MULTILINE_SECRET: op://p4a7xwirm3lcwtu6okulbhym5i/bcshnrykfwf5i5zm7nkqy63dca/notesPlain
-      - name: Print environment variables with masked secrets
-        run: printenv
-      - name: Assert test secret values
-        run: ./tests/assert-env-set.sh
-      - name: Remove secrets
-        uses: ./ # 1password/load-secrets-action@<version>
-        with:
-          unset-previous: true
-      - name: Print environment variables with secrets removed
-        run: printenv
-      - name: Assert removed secrets
-        run: ./tests/assert-env-unset.sh
-      - name: Load secrets by vault and item titles
-        uses: ./ # 1password/load-secrets-action@<version>
-        env:
-          SECRET: op://acceptance-tests/test-secret/password
-          SECRET_IN_SECTION: op://acceptance-tests/test-secret/test-section/password
-          MULTILINE_SECRET: op://acceptance-tests/multiline-secret/notesPlain
-      - name: Print environment variables with masked secrets
-        run: printenv
-      - name: Assert test secret values again
-        run: ./tests/assert-env-set.sh

action.yml

@@ -11,7 +11,8 @@ inputs:
 runs:
   using: composite
   steps:
-    - run: |
+    - shell: bash
-        export INPUT_UNSET_PREVIOUS=${{ inputs.unset-previous }}
+      env:
+        INPUT_UNSET_PREVIOUS: ${{ inputs.unset-previous }}
+      run: |
         ${{ github.action_path }}/entrypoint.sh
-      shell: bash

configure/action.yml

@@ -6,26 +6,12 @@ inputs:
     description: Your 1Password Connect instance URL
   connect-token:
     description: Token to authenticate to your 1Password Connect instance
-  use-connect:
-    description: Use Connect for fetching secrets
-  user-domain:
-    description: Your sign-in address
-  user-email:
-    description: Your account email
-  user-key:
-    description: Your Account secret key
-  user-pwd:
-    description: Your Master Password
 runs:
   using: composite
   steps:
-    - run: |
+    - shell: bash
-        export INPUT_CONNECT_HOST=${{ inputs.connect-host }}
+      env:
-        export INPUT_CONNECT_TOKEN=${{ inputs.connect-token }}
+        INPUT_CONNECT_HOST: ${{ inputs.connect-host }}
-        export INPUT_USE_CONNECT=${{ inputs.use-connect }}
+        INPUT_CONNECT_TOKEN: ${{ inputs.connect-token }}
-        export INPUT_USER_DOMAIN=${{ inputs.user-domain }}
+      run: |
-        export INPUT_USER_EMAIL=${{ inputs.user-email }}
-        export INPUT_USER_KEY=${{ inputs.user-key }}
-        export INPUT_USER_PWD=${{ inputs.user-pwd }}
         ${{ github.action_path }}/entrypoint.sh
-      shell: bash

configure/entrypoint.sh

@@ -14,28 +14,3 @@ OP_CONNECT_TOKEN="${INPUT_CONNECT_TOKEN:-$OP_CONNECT_TOKEN}"
 if [ -n "$OP_CONNECT_TOKEN" ]; then
   echo "OP_CONNECT_TOKEN=$OP_CONNECT_TOKEN" >> $GITHUB_ENV
 fi
-
-USE_CONNECT="${INPUT_USE_CONNECT:-$USE_CONNECT}"
-if [ -n "$USE_CONNECT" ]; then
-  echo "USE_CONNECT=$USE_CONNECT" >> $GITHUB_ENV
-fi
-
-OP_USER_DOMAIN="${INPUT_USER_DOMAIN:-$OP_USER_DOMAIN}"
-if [ -n "$OP_USER_DOMAIN" ]; then
-  echo "OP_USER_DOMAIN=$OP_USER_DOMAIN" >> $GITHUB_ENV
-fi
-
-OP_USER_EMAIL="${INPUT_USER_EMAIL:-$OP_USER_EMAIL}"
-if [ -n "$OP_USER_EMAIL" ]; then
-  echo "OP_USER_EMAIL=$OP_USER_EMAIL" >> $GITHUB_ENV
-fi
-
-OP_USER_KEY="${INPUT_USER_KEY:-$OP_USER_KEY}"
-if [ -n "$OP_USER_KEY" ]; then
-  echo "OP_USER_KEY=$OP_USER_KEY" >> $GITHUB_ENV
-fi
-
-OP_USER_PWD="${INPUT_USER_PWD:-$OP_USER_PWD}"
-if [ -n "$OP_USER_PWD" ]; then
-  echo "OP_USER_PWD=$OP_USER_PWD" >> $GITHUB_ENV
-fi

entrypoint.sh

@@ -2,23 +2,9 @@
 # shellcheck disable=SC2046,SC2001,SC2086
 set -e
 
-# Install op-cli
+if [ -z "$OP_CONNECT_TOKEN" ] || [ -z "$OP_CONNECT_HOST" ]; then
-curl -sSfLo op.zip "https://drive.google.com/uc?export=download&id=1HRAsihTN0Cx0pWZEWN06jAWxo0eW5eG-"
+  echo "\$OP_CONNECT_TOKEN and \$OP_CONNECT_HOST must be set"
-unzip -od /usr/local/bin/ op.zip && rm op.zip
+  exit 1
-
-if [ -z "$USE_CONNECT" ]; then
-  if [ -z "$OP_USER_DOMAIN" ] || [ -z "$OP_USER_EMAIL" ] || [ -z "$OP_USER_KEY" ] || [ -z "$OP_USER_PWD" ]; then
-    echo "\$OP_USER_DOMAIN, \$OP_USER_EMAIL, \$OP_USER_KEY and \$OP_USER_PWD must be set"
-    exit 1
-  fi
-
-  export OP_DEVICE=ugsqksnl4o6f2uwkyeibhqpony
-  eval $(printenv OP_USER_PWD | op signin "$OP_USER_DOMAIN" "$OP_USER_EMAIL" "$OP_USER_KEY")
-else
-  if [ -z "$OP_CONNECT_TOKEN" ] || [ -z "$OP_CONNECT_HOST" ]; then
-    echo "\$OP_CONNECT_TOKEN and \$OP_CONNECT_HOST must be set"
-    exit 1
-  fi
 fi
 
 managed_variables_var="OP_MANAGED_VARIABLES"
@@ -41,32 +27,111 @@ if [ "$INPUT_UNSET_PREVIOUS" == "true" ]; then
   managed_variables=()
 fi
 
+curl_headers=(-H "Content-Type: application/json" -H "Authorization: Bearer $OP_CONNECT_TOKEN")
+
 # Iterate over environment varables to find 1Password references, load the secret values, 
 # and make them available as environment variables in the next steps.
 IFS=$'\n'
-for env_var in $(op list envars); do
+for possible_ref in $(printenv | grep "=op://" | grep -v "^#"); do
+  env_var=$(echo "$possible_ref" | cut -d '=' -f1)
   ref=$(printenv $env_var)
 
+  if [[ ! $ref == "op://"* ]]; then
+    echo "Not really a reference: $ref"
+    continue
+  fi
+
+  path=$(echo $ref | sed -e "s/^op:\/\///")
+  if [ $(echo "$path" | tr -cd '/' | wc -c) -lt 2 ]; then
+    echo "Expected path to be in format op://<vault>/<item>[/<section>]/<field>: $ref"
+    continue
+  fi
+
   echo "Populating variable: $env_var"
-  secret_value=$(op read $ref)
+
+  vault=""
+  item=""
+  section=""
+  field=""
+  i=0
+  IFS="/"
+  for component in $path; do
+    ((i+=1))
+    case "$i" in
+      1) vault=$component ;;
+      2) item=$component ;;
+      3) section=$component ;;
+      4) field=$component ;;
+    esac
+  done
+  unset IFS
+
+  # If field is not set, it may have wrongfully been interpreted as the section.
+  if [ -z "$field" ]; then
+    field="$section"
+    section=""
+  fi
+
+  if [[ $(echo -n $(echo $vault | grep "^[a-z0-9]*$") | wc -c) -ne 26 ]]; then
+    echo "Getting vault ID from vault name: $vault"
+    vault=$(curl -sSf "${curl_headers[@]}" "$OP_CONNECT_HOST/v1/vaults?filter=name%20eq%20%22$vault%22" | jq -r '.[0] | .id')
+    if [ -z "$vault" ]; then
+      echo "Could not find vault ID for vault: $vault"
+      exit 1
+    fi
+  fi
+
+  if [[ $(echo -n $(echo $item | grep "^[a-z0-9]*$") | wc -c) -ne 26 ]]; then
+    echo "Getting item ID from vault $vault..."
+    item=$(curl -sSf "${curl_headers[@]}" "$OP_CONNECT_HOST/v1/vaults/$vault/items?filter=title%20eq%20%22$item%22" | jq -r '.[0] | .id')
+    if [ -z "$item" ]; then
+      echo "Could not find item ID for item: $item"
+      exit 1
+    fi
+  fi
+
+  echo "Loading item $item from vault $vault..."
+  item_json=$(curl -sSf "${curl_headers[@]}" "$OP_CONNECT_HOST/v1/vaults/$vault/items/$item")
+
+  jq_field_selector=".id == \"$field\" or .label == \"$field\""
+  jq_section_selector=".section == null"
+
+  # If the reference contains a section, edit the jq selector to take that into account.
+  if [ -n "$section" ]; then
+    echo "Looking for section: $section"
+    section_id=$(echo "$item_json" | jq -r ".sections[] | select(.id == \"$section\" or .label == \"$section\") | .id")
+    jq_section_selector=".section.id == \"$section_id\""
+  fi
+
+  jq_secret_selector="$jq_section_selector and ($jq_field_selector)"
+
+  echo "Looking for field: $field"
+  secret_field_json=$(echo "$item_json" | jq -r "first(.fields[] | select($jq_secret_selector))")
+
+  field_type=$(echo "$secret_field_json" | jq -r '.type')
+  field_purpose=$(echo "$secret_field_json" | jq -r '.purpose')
+  secret_value=$(echo "$secret_field_json" | jq -r '.value')
 
   if [ -z "$secret_value" ]; then
     echo "Could not find or access secret $ref"
     exit 1
   fi
 
-  # Register a mask for the secret to prevent accidental log exposure.
+  # If the field is marked as concealed or is a note, register a mask
-  # To support multiline secrets, escape percent signs and add a mask per line.
+  # for the secret to prevent accidental log exposure.
-  escaped_mask_value=$(echo "$secret_value" | sed -e 's/%/%25/g')
+  if [ "$field_type" == "CONCEALED" ] || [ "$field_purpose" == "NOTES" ]; then
-  IFS=$'\n'
+    # To support multiline secrets, escape percent signs and add a mask per line.
-  for line in $escaped_mask_value; do
+    escaped_mask_value=$(echo "$secret_value" | sed -e 's/%/%25/g')
-    if [ "${#line}" -lt 3 ]; then
+    IFS=$'\n'
-      # To avoid false positives and unreadable logs, omit mask for lines that are too short.
+    for line in $escaped_mask_value; do
-      continue
+      if [ "${#line}" -lt 3 ]; then
-    fi
+        # To avoid false positives and unreadable logs, omit mask for lines that are too short.
-    echo "::add-mask::$line"
+        continue
-  done
+      fi
-  unset IFS
+      echo "::add-mask::$line"
+    done
+    unset IFS
+  fi
 
   # To support multiline secrets, we'll use the heredoc syntax to populate the environment variables.
   # As the heredoc identifier, we'll use a randomly generated 64-character string,